diff --git a/docs/migration-pr.md b/docs/migration-pr.md new file mode 100644 index 0000000..94587ce --- /dev/null +++ b/docs/migration-pr.md @@ -0,0 +1,206 @@ +# Evidence-Backed Automated Migration PR (M9) + +`patchfrog/migration_pr/` is the final stage of **change -> impact -> +evidence -> verification -> decision**: given M8's evidence bundle, +**may PatchFrog open a PR at all, and if so, what exactly should it +contain?** `MIGRATION_PR_VERSION = 1`. No LLM ever decides eligibility, +branch identity, or PR content -- every field of a `MigrationPRPlan` is +derived from already-computed M6/M7/M8 evidence. This is specifically an +upstream-dependency-compatibility migration PR workflow, not a generic +coding-agent PR workflow. + +```bash +# Local dry-run: the exact branch/commit/PR/check plan a real publish +# would use, without ever touching GitHub. +python -m patchfrog.cli migrations publish --dry-run \ + --old old.yaml --new new.yaml --hints hints.yaml \ + --repo demo=path/to/repo --repository my-org/my-repo + +# The bundled demo, end to end through the dry-run PR dossier +python -m patchfrog.cli migrations demo --verified +``` + +Real publication (an actual GitHub write) requires a GitHub App +installation and is intentionally not reachable from this CLI in this +milestone -- see "Known limitations" below. + +## Domain model (`domain.py`) + +`MigrationPRLinkage` is the migration-PR analogue of M7's `PatchLinkage`: +upstream change fingerprint, dependency key, repository, base commit SHA, +plan/patch/verification-plan/bundle fingerprints. Its `identity_key()` is +deliberately narrower than the full linkage -- repository + change +fingerprint + engine version only -- so a *legitimately regenerated* +patch for the same upstream change reconciles the same PR (M9.6) rather +than creating a duplicate one. + +`MigrationPRPlan` is the complete, deterministic output of the +publication planner (`planner.py`): everything a dry-run needs to print, +and everything a real publish needs to write, with no further decisions +left to make at write time -- mirrors +`patchfrog.publishing.domain.ReviewPublicationPlan`'s own role for review +publishing. + +## Eligibility policy (M9.3, `eligibility.py`) + +A pure function of the M8 `VerificationOutcome` and an operator-controlled +`MigrationPRPolicy`. Never silently relaxed: + +| Verification outcome | Eligibility | +|---|---| +| `VERIFIED` | `AUTO_OPEN` | +| `PARTIALLY_VERIFIED` | `OPEN_WITH_OPERATOR_APPROVAL` only if `policy.allow_partially_verified` (off by default); otherwise `NOT_ELIGIBLE` | +| `HUMAN_REQUIRED` | `PLAN_ONLY` -- a report may exist, never an automatic code PR | +| `UNVERIFIED` / `FAILED` / `REGRESSION_DETECTED` | `NOT_ELIGIBLE`, unconditionally -- no policy knob relaxes these | + +A `PARTIALLY_VERIFIED` PR that is opened is always visibly marked in the +dossier (`dossier.py`'s own banner) -- the bar is never lowered silently. + +## Branch naming (M9.2, `branch.py`) + +``` +patchfrog/migrate// +``` + +Stable for the same migration identity (a re-run reconciles the same +branch, never a new one), sanitized to a git-ref-legal token, and falls +back to a stable `dependency` segment when no explicit provider identity +exists -- the branch shape is always exactly three path segments. The +branch name is only ever used as a JSON field value in a GitHub API +request body (`patchfrog.github.client`), never shell-interpolated, so +there is no command-injection surface. + +## Change Dossier (M9.4, `dossier.py`) + +A concise, evidence-backed PR body -- never raw logs, never AI prose. +Sections: Upstream change, Impact (blast radius summary), Migration +(strategies applied, files changed, human-required steps), Verification +(final outcome, targeted tests passed, contract checks, type check, +baseline-vs-patched evidence, evidence strength), Residual risk, and +Evidence identity (every fingerprint plus the repository base SHA). Ends +with an invisible HTML-comment marker (`marker.py`) carrying the +migration's `identity_key()` -- the same "sanitize untrusted text before +interpolation" discipline `patchfrog.publishing.marker` already uses, +even though dossier content here is deterministic evidence, not free-form +AI output. + +## GitHub Check integration (M9.5, `check.py`) + +Reuses `patchfrog.publishing.checks.CheckRunClient`'s Protocol and +`GitHubClient` verbatim -- a second check *name* +(`"PatchFrog Migration Verification"`) and external-id scheme, never a +second check-run publishing mechanism. `MigrationCheckPublisher.reconcile` +looks up an existing check run by name + external ID *for the given head +SHA* before creating a new one -- and since a GitHub check run always +lives on one exact commit, a regenerated patch (a new commit, because the +branch was force-moved) legitimately gets its own check run on its own +SHA; reconciliation only fires for a genuine retry of the *same* commit. + +## Publication sequence (M9.6/M9.7/M9.8, `publisher.py`) + +``` +MigrationPRPlan -> eligibility gate -> idempotency lookup (identity_key) + -> [DRY_RUN: stop here, no write] + -> stale-base check (M9.7) -> integrity check (M9.8, already enforced + earlier by the planner -- see below) + -> branch create/update -> PR open/update (M9.6) -> check run +``` + +Migration branches are entirely PatchFrog-owned: every publish rebuilds +the branch fresh from the *current* base commit and force-moves it -- +never a three-way merge, never preserving a human's own commits on it, +since nothing but PatchFrog's own generated content is ever expected to +live there. A durable row is written *before* any GitHub write is +attempted and re-locked after, mirroring +`patchfrog.publishing.service.ReviewPublicationService`'s own discipline +(GitHub and PatchFrog's database cannot share one transaction). + +### Idempotency (M9.6) + +One row per `identity_key` (`store.py`). Republishing the identical patch +content updates the same PR (never a duplicate); a regenerated patch +force-moves the same branch and updates the same PR; a PR closed by a +human is never reopened automatically (`NO_OP_UNCHANGED`). + +### Stale-base protection (M9.7) + +Before any write, the publisher reads the base branch's current head SHA +from GitHub and requires an *exact* match against +`linkage.base_commit_sha`. Any mismatch returns +`STALE_REQUIRES_REGENERATION` -- a moved base means the plan/patch/ +evidence were computed against a repository state that no longer exists; +PatchFrog never publishes a stale `VERIFIED` status against a different +base. + +### Evidence/patch integrity (M9.8, `integrity.py`) + +Enforced even earlier than publication -- `planner.build_pr_plan` calls +`verify_evidence_integrity` before it builds anything, raising +`MigrationPRIntegrityError` if the evidence bundle's own +`change_fingerprint`/`patch_fingerprint`/`repository_head_sha` do not +correspond exactly to the change/patch/base-commit-SHA the plan is about +to be built for. A `VERIFIED` bundle computed for one patch can therefore +never be silently reused for a different one -- a caller passing +mismatched arguments gets a loud failure, not a quietly wrong plan. + +## Testing (M9.9, `fake_github.py`) + +`FakeMigrationGitHubPublisher` satisfies the `MigrationGitHubPublisher` +Protocol structurally -- a legitimate stand-in for tests, not a mock of +internal plumbing, and no real GitHub PR is ever opened by an automated +test. `tests/integration/test_migration_pr_publisher.py` covers: dry-run +makes no GitHub call, a verified migration opens a PR, republishing +identical/regenerated patches reconciles the same PR, stale base never +publishes, failed/regression/unverified outcomes never open a PR, +partially-verified respects operator policy, human-required never +touches GitHub, a closed PR is never reopened, and check-run +reconciliation is per-commit-SHA (a regenerated patch's new commit +legitimately gets its own check run; a retried identical commit +reconciles the existing one). + +## Local dry-run (M9.10, `report.py`, `patchfrog migrations publish`) + +Builds the exact `MigrationPRPlan` a real publish would use and renders +it -- proposed branch, commit title, PR title/body, check title/summary, +and whether policy allows publication -- without ever calling GitHub. A +migration PR targets exactly one repository, so `publish` takes exactly +one `--repo NAME=PATH` plus `--repository OWNER/REPO` (the GitHub +identity to publish against); `--allow-partially-verified` maps directly +to `MigrationPRPolicy.allow_partially_verified`. + +## Persistence (`store.py`, migration `0037_migration_pull_requests`) + +`migration_pull_requests` -- one bounded row per `identity_key`. Chained +after `0036_migration_verification`. Never a duplicate row for the same +upstream change; a repeated publish always updates the existing row's +patch/verification/PR-state fields. + +## Known limitations + +- Real GitHub publication (an actual branch/commit/PR write) is only + exercised in this milestone against the fake adapter in tests; the CLI + intentionally only exposes `--dry-run`. `GitHubClientMigrationPublisher` + (`github_publisher.py`) already wires `MigrationPRPublisher` to a real + installation-scoped `GitHubClient` for a future production caller; + wiring that into the webhook/worker pipeline is future work. An opt-in + real-GitHub E2E harness exists (M9.11, + `tests/integration/test_migration_pr_real_github_e2e.py`, gated on + `PATCHFROG_MIGRATION_PR_REAL_GITHUB_E2E=1`) but is deliberately left + unimplemented and is never run in this milestone or in CI. +- Stale-base protection only checks the base branch's head SHA at + publish time -- a base that moves *during* the publish sequence itself + (between the ref read and the tree/commit write) is not separately + re-checked; GitHub's own ref-update semantics (an unforced `update_ref` + would fail on a non-fast-forward) are the last line of defense there. +- The dossier's blast-radius section summarizes per-dependency counts + only; the full per-symbol detail lives in the M6 blast radius object + itself, not duplicated into the PR body (deliberately, to keep the + dossier concise per M9.4's own instruction). + +## Deliberately out of scope + +A generic coding-agent PR workflow of any kind; auto-merging the +generated migration PR (never done, regardless of verification outcome); +hosted-business concerns (accounts, billing, provider routing policy, +GitHub App identity) -- those belong to PatchFrog Cloud, never this +package, per `docs/product-boundary.md`. diff --git a/docs/migration-verification.md b/docs/migration-verification.md new file mode 100644 index 0000000..c8883c6 --- /dev/null +++ b/docs/migration-verification.md @@ -0,0 +1,195 @@ +# Migration Verification Lab (M8) + +`patchfrog/migration_verification/` answers the executable-evidence stage +of **change -> impact -> evidence -> verification -> decision**: given +M7's migration plan and generated patch, **what evidence would actually +show this migration restores compatibility, and what did PatchFrog +actually gather?** `MIGRATION_VERIFICATION_VERSION = 1`. Never calls a +provider -- `test_migration_verification_and_pr_modules_never_import_a_provider` +(`tests/unit/test_migration_verification_and_pr_module_boundaries.py`) +enforces this the same way +`test_deterministic_migration_modules_never_import_a_provider` already does +for M7. + +A passing generic test, or a syntax/type check alone, is never called +`VERIFIED`. The decision engine (M8.11) is deterministic and explainable; +no LLM ever decides final truth. + +```bash +# Bounded, sandboxed verification of a generated migration patch +python -m patchfrog.cli migrations verify --old old.yaml --new new.yaml \ + --hints hints.yaml --repo demo=path/to/repo + +# The bundled demos, extended through M8 (see docs/migration-pr.md for M9) +python -m patchfrog.cli migrations demo # human-required path +python -m patchfrog.cli migrations demo --verified # fully-automatic, VERIFIED path +``` + +## Pipeline (`service.py`) + +``` +plan + patch + blast radii + exact repository state + -> requirements (requirements.py) -- M8.2 + -> targeted test selection (test_selection.py) -- M8.3 + -> bounded execution plan (execution_plan.py) -- M8.4 + -> sandboxed execution (via executable_verification.sandbox) -- M8.5 + -> baseline-vs-patched comparison (baseline.py) -- M8.6 + -> contract verification (contract.py) -- M8.7 + -> regression detection (regression.py) -- M8.8 + -> evidence bundle (evidence.py) -- M8.9/M8.10 + -> decision (decision.py) -- M8.11 +``` + +`run_migration_verification` is safe to call for any migration result, +including one with no patch at all (a plan that is entirely +`HUMAN_REQUIRED`): it returns an evidence-free bundle immediately, no +sandbox touched. + +## Requirement generation and targeted test selection (M8.2/M8.3) + +Requirements are generated only from what the migration actually claims: +syntax validity, import resolution, a type check when the repository has +one configured, contract-specific checks per M6 diff item kind, and -- +only when consumer-behavior evidence is possible -- targeted unit/ +integration tests. The plan is bounded (`MAX_VERIFICATION_STEPS_PER_PLAN`, +`MAX_TARGETED_TESTS_PER_PLAN`); PatchFrog never runs "everything" when +targeted evidence is enough. + +Test selection reuses M8's own affected-symbol/blast-radius evidence +(`BlastRadius.related_tests`) and the caller graph, and classifies every +selection: + +| Classification | Meaning | +|---|---| +| `DIRECT` | a test file already known to reference the affected symbol/usage site | +| `TRANSITIVE` | reached through the caller graph, not a direct reference | +| `FALLBACK` | no direct/transitive evidence; a broader, lower-confidence net | +| `UNKNOWN` | selection exists but confidence could not be established | + +Every selection carries its own `reason` -- the mapping is never claimed +perfect. + +## Safe execution boundary (M8.5) + +Every verification step runs through +`patchfrog.executable_verification.sandbox.VerificationSandbox` (`bwrap` + +`prlimit`) -- the same primitive Executable Verification already uses for +finding-fix verification; M8 adds no second sandbox. Fixed working +directory, bounded timeout, bounded stdout/stderr capture, no network by +default, no secret values injected, no git push, no package-publish/ +deploy command, process termination on timeout. Where the sandbox is +unavailable on a host, every step comes back `CheckStatus.UNAVAILABLE` +with an honest `detail` -- never silently skipped, never guessed passing. + +## Baseline-vs-patched comparison (M8.6) + +Bounded to `MAX_BASELINE_COMPARISONS` (2) `DIRECT`/`TRANSITIVE` selections +only -- `FALLBACK`/`UNKNOWN` selections are heuristic-strength at best and +never worth spending a second sandbox run on. Reuses +`executable_verification.snapshot_staging` to export the baseline +(pre-migration) commit state and run the identical check against it, then +against the patched workspace: + +| Baseline | Patched | Outcome | +|---|---|---| +| fail | pass | `BASELINE_FAIL_PATCHED_PASS` -- strong fix evidence | +| pass | pass | compatibility evidence, not proof of an original failure | +| fail | fail (differently) | unresolved | +| pass | fail | regression | +| unavailable | -- | evidence weaker; said so explicitly, never guessed | + +## Contract verification (M8.7) + +Reuses M6's own `ContractDiffItem`/`DiffItemKind` evidence and M7's +`PatchLinkage` directly -- never re-derives them, never relies on string +matching alone where parser/symbol information already exists (the same +AST-based location M7's rewriters use). + +## Regression detection (M8.8) + +Distinct from `UNVERIFIED`: a migration that introduces new damage +(syntax failure, type failure, a targeted-test regression, an obsolete +symbol newly introduced, a dependency conflict, a contract mismatch, a +safety-gate regression) is `REGRESSION_DETECTED`, never merely +"insufficient evidence." + +## Evidence bundle and strength (M8.9/M8.10) + +`MigrationEvidenceBundle` is first-class, reproducible, and +machine-readable: upstream change fingerprint, repository + exact head +SHA, patch fingerprint, the verification plan (with its own +`fingerprint()`), every step's evidence, contract checks, baseline +comparisons, coverage (satisfied/failed/not-run/unavailable, split from +mandatory), evidence strength, residual risk, and the final outcome with +explicit reasons. Only bounded summaries, hashes, exit status and timing +are persisted -- never raw logs beyond a short bounded excerpt, never +secrets. + +`bundle_fingerprint` ties the bundle to an *exact* patch/change/repository +state -- see M9.8's integrity check in `docs/migration-pr.md`, which +enforces that a `VERIFIED` bundle for one patch is never reused for +another. + +| Evidence strength | Example | +|---|---| +| `STRONG` | baseline-fail/patched-pass on a direct check; all mandatory contract checks passed | +| `MODERATE` | a directly-related test/type check passes on the patched state alone | +| `WEAK` | a heuristic/fallback-only signal, or many generic passes with no direct relation | +| `NONE` | nothing gathered (no patch, or no requirements at all) | + +Many weak signals never combine into `STRONG` -- see +`test_many_weak_signals_never_promote_to_strong`. + +## Decision engine (M8.11) + +Mirrors the evidence-combination discipline +`patchfrog.fix_verification.domain.FixEvidenceDirection` reached only +after two real correction rounds (`validation/agent_handoff/latest-summary.md`): +a single strong contradicting signal (a regression, a failed mandatory +contract check) always wins outright; only strong, direct evidence may +reach `VERIFIED`; any combination of only weak/generic evidence never +promotes above `PARTIALLY_VERIFIED`; missing required evidence is +`UNVERIFIED`, never guessed in either direction. + +| Outcome | Meaning | +|---|---| +| `VERIFIED` | all mandatory requirements satisfied, no regression, sufficient evidence strength | +| `PARTIALLY_VERIFIED` | important checks pass, some required evidence unavailable, no known regression | +| `UNVERIFIED` | insufficient evidence | +| `FAILED` | the migration does not restore required compatibility | +| `REGRESSION_DETECTED` | the migration introduces a new failure | +| `HUMAN_REQUIRED` | the migration plan itself has an unresolved human-required step | + +## Persistence (`store.py`, migration `0036_migration_verification`) + +`migration_verification_runs` -- one bounded row per (patch, bundle +fingerprint); a repeated identical run is idempotent (re-verifying does +not create a duplicate row). Chained after `0035_upstream_change_migration`. + +## Fix Verification is deliberately not reused + +`patchfrog.fix_verification` is finding-centric (keyed by `handoff_id`, +answers "does this fix a specific reported finding"). Migration +Verification answers a different question ("does this migration restore +compatibility with an upstream change") over a different identity +(change/patch fingerprint). Only the evidence-combination *discipline* is +shared; the enums, modules, and persistence are deliberately separate, per +this package's own instruction not to overload existing review-status +enums with a second, unrelated meaning. + +## Known limitations + +- Baseline comparisons only run for `DIRECT`/`TRANSITIVE` test selections, + bounded to 2 per verification -- a migration with many affected call + sites gets baseline proof for its strongest evidence only, not every site. +- Where the sandbox itself is unavailable on a host, every step is + reported `UNAVAILABLE` rather than run -- this is honest, bounded + degradation, not a silent pass. +- Type checking only runs where the target repository already has mypy + configured; PatchFrog never invents a type-checker configuration. + +## Deliberately out of scope + +Opening a PR with the verified migration (M9, see `docs/migration-pr.md`); +any verification requirement without a deterministic evidence source +(left `UNVERIFIED`/`HUMAN_REQUIRED`, never guessed). diff --git a/docs/migration.md b/docs/migration.md index ea65b4a..9ed45f8 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -6,8 +6,9 @@ change and consumer impact, **what exactly must each affected call site do, can it be fixed automatically, and if so, what is the literal patch?** `MIGRATION_ENGINE_VERSION = 1`. "Verification" here means deterministic structural/contract validation only (the safety gates below) -- -executable/runtime verification is M8 and automated migration PRs are -M9; neither is implemented in this milestone. +executable/runtime verification is M8 (`docs/migration-verification.md`) +and automated migration PRs are M9 (`docs/migration-pr.md`), both now +implemented as the two stages after this one. ```bash # Plan only (one repository, or several with --repo NAME=PATH, repeatable) @@ -121,7 +122,8 @@ check key on. ## Deliberately out of scope -Executable/runtime verification that a generated patch actually fixes -the break (M8); opening a PR with the generated patch (M9); any -migration strategy without a deterministic or hint/model-sourced value -(left `HUMAN_REQUIRED`, never guessed). +Any migration strategy without a deterministic or hint/model-sourced +value (left `HUMAN_REQUIRED`, never guessed). Executable/runtime +verification that a generated patch actually fixes the break, and +opening a PR with the generated patch, are the next two stages -- +see `docs/migration-verification.md` (M8) and `docs/migration-pr.md` (M9). diff --git a/docs/roadmap.md b/docs/roadmap.md index bf0e9c1..d107c6f 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -212,16 +212,24 @@ structured value field, same rewriter and safety gates, its own `PatchOrigin`, never merged with the deterministic patch); and idempotent persistence with a `PatchLinkage` M8/M9 can key on. See `docs/migration.md`. -**M8 — Executable/Runtime Verification of a generated migration.** Next; -**not started.** Wire a generated M7 patch into (an extension of) the -existing executable/fix-verification sandbox to confirm the original -break is actually gone at the candidate fix, not just structurally -plausible. - -**M9 — Automated Migration PRs.** Not started; depends on M8. Open an -evidence-backed pull request from a verified M7 patch, through the -existing publishing/merge-readiness surface -- never a second PR-opening -path. +**M8 — Migration Verification Lab.** Implemented: a deterministic +requirement/targeted-test-selection/execution-plan pipeline that wires a +generated M7 patch into the existing sandboxed execution boundary, +bounded baseline-vs-patched comparison, contract-specific verification +reusing M6's own diff evidence, regression detection distinct from mere +insufficient evidence, a reproducible evidence bundle with explicit +evidence strength, and a deterministic (never LLM-decided) outcome +engine. See `docs/migration-verification.md`. + +**M9 — Evidence-Backed Automated Migration PRs.** Implemented: a +publication-eligibility policy keyed on the M8 outcome (never silently +relaxed), deterministic branch naming and an evidence-backed Change +Dossier, idempotent publication through the existing GitHub Check +infrastructure (never a second publishing system), stale-base and +evidence/patch integrity protection, and a local `--dry-run` CLI. Real +GitHub publication is wired (`GitHubClientMigrationPublisher`) but not +yet called from the production webhook/worker pipeline -- see +`docs/migration-pr.md`'s "Known limitations". ## Then — Advanced Verification diff --git a/migrations/versions/0036_migration_verification.py b/migrations/versions/0036_migration_verification.py new file mode 100644 index 0000000..b001f30 --- /dev/null +++ b/migrations/versions/0036_migration_verification.py @@ -0,0 +1,46 @@ +"""M8 migration verification runs + +Revision ID: 0036_migration_verification +Revises: 0035_upstream_change_migration +Create Date: 2026-09-26 + +""" +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0036_migration_verification" +down_revision: str | None = "0035_upstream_change_migration" +branch_labels: Sequence[str] | None = None +depends_on: Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "migration_verification_runs", + sa.Column("id", sa.Uuid(), primary_key=True), + sa.Column("patch_id", sa.Uuid(), sa.ForeignKey("migration_patches.id", ondelete="CASCADE"), nullable=False), + sa.Column("change_fingerprint", sa.String(length=64), nullable=False), + sa.Column("patch_fingerprint", sa.String(length=64), nullable=False), + sa.Column("verification_plan_fingerprint", sa.String(length=64), nullable=False), + sa.Column("engine_version", sa.Integer(), nullable=False), + sa.Column("repository_head_sha", sa.String(length=64), nullable=False, server_default=""), + sa.Column("outcome", sa.String(length=24), nullable=False), + sa.Column("evidence_strength", sa.String(length=16), nullable=False), + sa.Column("residual_risk", sa.String(length=16), nullable=False), + sa.Column("bundle_json", sa.Text(), nullable=False, server_default="{}"), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_verified_at", sa.DateTime(timezone=True), nullable=False), + sa.UniqueConstraint( + "patch_id", "verification_plan_fingerprint", name="uq_migration_verification_runs_identity" + ), + ) + op.create_index("ix_migration_verification_runs_patch", "migration_verification_runs", ["patch_id"]) + + +def downgrade() -> None: + op.drop_index("ix_migration_verification_runs_patch", table_name="migration_verification_runs") + op.drop_table("migration_verification_runs") diff --git a/migrations/versions/0037_migration_pull_requests.py b/migrations/versions/0037_migration_pull_requests.py new file mode 100644 index 0000000..d2ccb10 --- /dev/null +++ b/migrations/versions/0037_migration_pull_requests.py @@ -0,0 +1,48 @@ +"""M9 migration pull request publication + +Revision ID: 0037_migration_pull_requests +Revises: 0036_migration_verification +Create Date: 2026-09-26 + +""" +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0037_migration_pull_requests" +down_revision: str | None = "0036_migration_verification" +branch_labels: Sequence[str] | None = None +depends_on: Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "migration_pull_requests", + sa.Column("id", sa.Uuid(), primary_key=True), + sa.Column("identity_key", sa.String(length=64), nullable=False), + sa.Column("repository", sa.String(length=512), nullable=False), + sa.Column("change_fingerprint", sa.String(length=64), nullable=False), + sa.Column("engine_version", sa.Integer(), nullable=False), + sa.Column("branch_name", sa.String(length=255), nullable=False), + sa.Column("patch_fingerprint", sa.String(length=64), nullable=False), + sa.Column("verification_plan_fingerprint", sa.String(length=64), nullable=False), + sa.Column("bundle_fingerprint", sa.String(length=64), nullable=False), + sa.Column("base_commit_sha", sa.String(length=64), nullable=False), + sa.Column("eligibility", sa.String(length=32), nullable=False), + sa.Column("status", sa.String(length=32), nullable=False), + sa.Column("pr_number", sa.Integer(), nullable=True), + sa.Column("pr_html_url", sa.String(length=1024), nullable=True), + sa.Column("reason", sa.Text(), nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_reconciled_at", sa.DateTime(timezone=True), nullable=False), + sa.UniqueConstraint("identity_key", name="uq_migration_pull_requests_identity"), + ) + op.create_index("ix_migration_pull_requests_repository", "migration_pull_requests", ["repository"]) + + +def downgrade() -> None: + op.drop_index("ix_migration_pull_requests_repository", table_name="migration_pull_requests") + op.drop_table("migration_pull_requests") diff --git a/patchfrog/cli_changes.py b/patchfrog/cli_changes.py index 96c43f0..fb4cee9 100644 --- a/patchfrog/cli_changes.py +++ b/patchfrog/cli_changes.py @@ -38,6 +38,13 @@ render_plan_text, ) from patchfrog.migration.store import MigrationStore, build_linkage +from patchfrog.migration_pr.domain import MigrationPRPolicy +from patchfrog.migration_pr.planner import build_pr_plan +from patchfrog.migration_pr.report import pr_plan_to_dict, render_pr_plan_text +from patchfrog.migration_verification.domain import MigrationEvidenceBundle +from patchfrog.migration_verification.report import bundle_to_dict, render_bundle_text +from patchfrog.migration_verification.service import run_migration_verification +from patchfrog.migration_verification.store import MigrationVerificationStore from patchfrog.persistence.database import create_engine, create_session_factory from patchfrog.persistence.models.repository import RepositoryModel from patchfrog.repository.git import GitError, run_git @@ -481,6 +488,139 @@ def run_migrations_generate(args: argparse.Namespace, upsert_repository: UpsertR return 0 +async def _persist_verification( + *, + event: ExternalChangeEvent, + plan: MigrationPlan, + patch: GeneratedPatch | None, + bundle: MigrationEvidenceBundle | None, + root: Path, + repository_name: str, + upsert_repository: UpsertRepository, +) -> dict[str, Any]: + result = await _persist_plan_and_patch( + event=event, plan=plan, patch=patch, root=root, repository_name=repository_name, + upsert_repository=upsert_repository, + ) + if patch is None or bundle is None or "patch_id" not in result: + return result + + settings = get_settings() + engine = create_engine(settings.database_url) + session_factory: async_sessionmaker[AsyncSession] = create_session_factory(engine) + verification_store = MigrationVerificationStore() + try: + async with session_factory() as session: + run_row, run_created = await verification_store.record_run( + session, patch_id=uuid.UUID(result["patch_id"]), bundle=bundle + ) + await session.commit() + result["verification_run_id"] = str(run_row.id) + result["verification_run_created"] = run_created + return result + finally: + await engine.dispose() + + +def run_migrations_verify(args: argparse.Namespace, upsert_repository: UpsertRepository) -> int: + if not args.repo: + raise CommandError("give at least one --repo PATH (or NAME=PATH)") + analysis = analyze_local(args, [_repo_arg(r) for r in args.repo]) + plans: dict[str, MigrationPlan] = {} + patches: dict[str, GeneratedPatch | None] = {} + bundles: dict[str, MigrationEvidenceBundle] = {} + for impact in analysis.workspace.repositories: + inventory = analysis.inventories[impact.repository] + plan = plan_migration(analysis.event, impact, inventory, hints=analysis.hints) + patch = generate_patch(plan, analysis.roots[impact.repository]) if plan.automatic_steps else None + plans[impact.repository] = plan + patches[impact.repository] = patch + bundles[impact.repository] = asyncio.run( + run_migration_verification( + plan=plan, patch=patch, blast_radii=impact.blast_radii, root=analysis.roots[impact.repository], + commit_sha=impact.commit_sha or "", + ) + ) + + persisted: dict[str, Any] = {} + if args.persist: + for name, plan in plans.items(): + persisted[name] = asyncio.run( + _persist_verification( + event=analysis.event, plan=plan, patch=patches[name], bundle=bundles[name], + root=analysis.roots[name], repository_name=name, upsert_repository=upsert_repository, + ) + ) + + payload: dict[str, Any] = { + "event": event_to_dict(analysis.event), + "verification": {name: bundle_to_dict(bundle) for name, bundle in bundles.items()}, + } + if persisted: + payload["persisted"] = persisted + if args.output: + Path(args.output).write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") + if args.json: + print(json.dumps(payload, indent=2, sort_keys=True)) + else: + print(render_event_text(analysis.event), end="") + for name, bundle in bundles.items(): + print(f"Migration verification for {name}:") + print(render_bundle_text(bundle), end="") + if persisted: + print(f"persisted: {persisted}") + return 0 + + +def run_migrations_publish(args: argparse.Namespace) -> int: + """M9.10: local dry-run of migration PR publication. Builds the exact + :class:`~patchfrog.migration_pr.domain.MigrationPRPlan` a real publish + would use (M9.1--M9.8) and renders it -- proposed branch, commit + title, PR title/body, check outcome, and whether policy allows + publication -- without ever touching GitHub. Real publication (a + GitHub write) is out of scope for this CLI in this milestone; see + ``docs/migration-pr.md``.""" + + if not args.dry_run: + raise CommandError( + "only --dry-run is supported by this CLI in this milestone -- real publication " + "requires a GitHub App installation and is not available offline (see docs/migration-pr.md)" + ) + if not args.repo: + raise CommandError("give exactly one --repo NAME=PATH -- a migration PR targets one repository") + if len(args.repo) > 1: + raise CommandError("give exactly one --repo -- a migration PR targets one repository, not a fleet") + if not args.repository: + raise CommandError("--repository OWNER/REPO is required (the GitHub identity to publish against)") + + analysis = analyze_local(args, [_repo_arg(r) for r in args.repo]) + impact = analysis.workspace.repositories[0] + inventory = analysis.inventories[impact.repository] + root = analysis.roots[impact.repository] + plan = plan_migration(analysis.event, impact, inventory, hints=analysis.hints) + patch = generate_patch(plan, root) if plan.automatic_steps else None + base_commit_sha = impact.commit_sha or "" + bundle = asyncio.run( + run_migration_verification( + plan=plan, patch=patch, blast_radii=impact.blast_radii, root=root, commit_sha=base_commit_sha, + ) + ) + policy = MigrationPRPolicy(allow_partially_verified=args.allow_partially_verified) + pr_plan = build_pr_plan( + event=analysis.event, plan=plan, patch=patch, bundle=bundle, blast_radii=impact.blast_radii, + repository=args.repository, base_commit_sha=base_commit_sha, policy=policy, + ) + + payload = pr_plan_to_dict(pr_plan) + if args.output: + Path(args.output).write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") + if args.json: + print(json.dumps(payload, indent=2, sort_keys=True)) + else: + print(render_pr_plan_text(pr_plan), end="") + return 0 + + #: The M7.10 demo: a deterministic, fictional SDK #: (``client.chat.create(prompt=...)`` -> ``client.responses.create(input=...)``) #: bundled with the repository -- offline, no live provider call, never @@ -490,8 +630,64 @@ def run_migrations_generate(args: argparse.Namespace, upsert_repository: UpsertR #: uses for `eval run`. DEMO_ROOT = Path(__file__).resolve().parent.parent / "tests" / "fixtures" / "upstream_changes" / "demo" +#: M9.12: the same fictional ``acme-ai`` SDK, but with a single, +#: fully-automatic consumer and a bundled 2.0-shaped SDK stub -- the +#: "everything checks out" counterpart to ``DEMO_ROOT``'s one +#: human-required step. See ``tests/fixtures/upstream_changes/demo_verified``. +DEMO_VERIFIED_ROOT = Path(__file__).resolve().parent.parent / "tests" / "fixtures" / "upstream_changes" / "demo_verified" + +#: A stable, fictional repository head for the bundled demo only -- never +#: a real commit. Keeps M8 evidence (``repository_head_sha``) and M9 +#: linkage (``base_commit_sha``) trivially consistent for M9.8's own +#: integrity check without needing a real git checkout. +DEMO_COMMIT_SHA = "d3adb33f" * 5 +DEMO_REPOSITORY = "patchfrog-demo/acme-ai-consumer" + + +def _demo_verified_pipeline() -> tuple[ + ExternalChangeEvent, MigrationPlan, GeneratedPatch, MigrationEvidenceBundle, tuple[Any, ...] +]: + if not DEMO_VERIFIED_ROOT.is_dir(): + raise CommandError(f"demo assets not found at {DEMO_VERIFIED_ROOT} (expected a full repository checkout)") + hints = load_hints((DEMO_VERIFIED_ROOT / "hints.yaml").read_text()) + event = build_contract_change( + load_contract_file(DEMO_VERIFIED_ROOT / "old.yaml"), load_contract_file(DEMO_VERIFIED_ROOT / "new.yaml"), + hints=hints, + ) + root = DEMO_VERIFIED_ROOT / "repo" + inventory = discover_dependencies(root, repository="demo", adapters=adapters_for_target(event.target)) + impact = analyze_inventory(inventory, event, hints=hints, root=root) + plan = plan_migration(event, impact, inventory, hints=hints) + patch = generate_patch(plan, root) + bundle = asyncio.run( + run_migration_verification( + plan=plan, patch=patch, blast_radii=impact.blast_radii, root=root, commit_sha=DEMO_COMMIT_SHA, + extra_workspace_paths=(DEMO_VERIFIED_ROOT / "sdk_stub",), + ) + ) + return event, plan, patch, bundle, impact.blast_radii + def run_migrations_demo(args: argparse.Namespace) -> int: + if getattr(args, "verified", False): + event, plan, patch, bundle, blast_radii = _demo_verified_pipeline() + pr_plan = build_pr_plan( + event=event, plan=plan, patch=patch, bundle=bundle, blast_radii=blast_radii, + repository=DEMO_REPOSITORY, base_commit_sha=DEMO_COMMIT_SHA, + ) + if args.json: + print(json.dumps({ + "event": event_to_dict(event), "plan": plan_to_dict(plan), "patch": patch_to_dict(patch), + "verification": bundle_to_dict(bundle), "pr_plan": pr_plan_to_dict(pr_plan), + }, indent=2, sort_keys=True)) + return 0 + print(render_event_text(event), end="") + print(render_plan_text(plan), end="") + print(render_patch_text(patch), end="") + print(render_bundle_text(bundle), end="") + print(render_pr_plan_text(pr_plan), end="") + return 0 + if not DEMO_ROOT.is_dir(): raise CommandError(f"demo assets not found at {DEMO_ROOT} (expected a full repository checkout)") hints = load_hints((DEMO_ROOT / "hints.yaml").read_text()) @@ -503,13 +699,32 @@ def run_migrations_demo(args: argparse.Namespace) -> int: plan = plan_migration(event, impact, inventory, hints=hints) patch = generate_patch(plan, root) + # M9.12: also demonstrate the non-verified path -- this fixture's one + # `human_required remove_argument` step means M8/M9 can never reach + # more than HUMAN_REQUIRED / PLAN_ONLY here, however cleanly the rest + # checks out (see docs/migration-pr.md). + bundle = asyncio.run( + run_migration_verification( + plan=plan, patch=patch, blast_radii=impact.blast_radii, root=root, commit_sha=DEMO_COMMIT_SHA, + extra_workspace_paths=(DEMO_ROOT / "sdk_stub",), + ) + ) + pr_plan = build_pr_plan( + event=event, plan=plan, patch=patch, bundle=bundle, blast_radii=impact.blast_radii, + repository=DEMO_REPOSITORY, base_commit_sha=DEMO_COMMIT_SHA, + ) + if args.json: - print(json.dumps({"event": event_to_dict(event), "plan": plan_to_dict(plan), - "patch": patch_to_dict(patch)}, indent=2, sort_keys=True)) + print(json.dumps({ + "event": event_to_dict(event), "plan": plan_to_dict(plan), "patch": patch_to_dict(patch), + "verification": bundle_to_dict(bundle), "pr_plan": pr_plan_to_dict(pr_plan), + }, indent=2, sort_keys=True)) return 0 print(render_event_text(event), end="") print(render_plan_text(plan), end="") print(render_patch_text(patch), end="") + print(render_bundle_text(bundle), end="") + print(render_pr_plan_text(pr_plan), end="") return 0 @@ -572,10 +787,40 @@ def register(subparsers: Any) -> None: generate.add_argument("--json", action="store_true") generate.add_argument("--output", default=None) + verify = migrations_sub.add_parser( + "verify", + help="M8: bounded, sandboxed executable verification of a generated migration patch (offline)", + ) + add_change_inputs(verify) + verify.add_argument("--repo", action="append", default=[], + help="Local repository checkout (repeatable; NAME=PATH to name it)") + verify.add_argument("--persist", action="store_true", + help="Record the event, plan, patch and verification run (requires DATABASE_URL)") + verify.add_argument("--json", action="store_true") + verify.add_argument("--output", default=None) + + publish = migrations_sub.add_parser( + "publish", + help="M9.10: local dry-run of migration PR publication -- branch/commit/PR/check plan, no GitHub write", + ) + add_change_inputs(publish) + publish.add_argument("--repo", action="append", default=[], + help="Local repository checkout (NAME=PATH) -- exactly one, a migration PR targets one repository") + publish.add_argument("--repository", help="GitHub repository identity to publish against (owner/repo)") + publish.add_argument("--dry-run", action="store_true", + help="Required in this milestone -- real publication is not available from this CLI") + publish.add_argument("--allow-partially-verified", action="store_true", + help="Operator policy: allow a PARTIALLY_VERIFIED migration to open a visibly-marked PR") + publish.add_argument("--json", action="store_true") + publish.add_argument("--output", default=None) + demo = migrations_sub.add_parser( - "demo", help="M7.10: the bundled deterministic demo, end to end (offline, no live provider call)" + "demo", help="M7.10/M9.12: the bundled deterministic demo, end to end (offline, no live provider call)" ) demo.add_argument("--json", action="store_true") + demo.add_argument("--verified", action="store_true", + help="Run the fully-automatic demo_verified scenario (VERIFIED) instead of the default " + "human-required scenario, and extend it through M8 verification and the M9 dry-run PR dossier") def dispatch(args: argparse.Namespace, *, upsert_repository: UpsertRepository) -> int: @@ -590,6 +835,10 @@ def dispatch(args: argparse.Namespace, *, upsert_repository: UpsertRepository) - return run_migrations_plan(args, upsert_repository) if args.migrations_command == "generate": return run_migrations_generate(args, upsert_repository) + if args.migrations_command == "verify": + return run_migrations_verify(args, upsert_repository) + if args.migrations_command == "publish": + return run_migrations_publish(args) if args.migrations_command == "demo": return run_migrations_demo(args) except (CommandError, ContractLoadError, SurfaceError, HintError) as exc: diff --git a/patchfrog/domain/github_git.py b/patchfrog/domain/github_git.py new file mode 100644 index 0000000..219f392 --- /dev/null +++ b/patchfrog/domain/github_git.py @@ -0,0 +1,23 @@ +"""Typed GitHub Git Data API transport models -- the primitives M9 needs +to build a migration branch/commit without ever shelling out to a local +``git`` binary (no clone, no credential-bearing remote URL, no local +working tree at all: every write is a plain, typed GitHub API call).""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True, slots=True) +class GitTreeEntry: + """One file's desired content in a new commit tree. ``content`` is + the file's full new text content -- GitHub's Git Data API creates the + backing blob server-side, so no separate blob-creation round trip is + needed for text files.""" + + path: str + content: str + mode: str = "100644" + + +__all__ = ["GitTreeEntry"] diff --git a/patchfrog/github/client.py b/patchfrog/github/client.py index ed28bc7..cbff26a 100644 --- a/patchfrog/github/client.py +++ b/patchfrog/github/client.py @@ -28,6 +28,7 @@ GitHubReviewComment, GitHubReviewThreadStatus, ) +from patchfrog.domain.github_git import GitTreeEntry from patchfrog.domain.github_review import ( GitHubReviewCommentInput, GitHubReviewEvent, @@ -113,6 +114,160 @@ async def get_default_branch_head_sha( raise GitHubResponseError(f"GitHub did not report a head commit for {owner}/{repository}@{default_branch}") return sha + async def get_ref(self, *, installation_id: int, owner: str, repository: str, ref: str) -> str | None: + """The commit SHA a ref (e.g. ``heads/main`` or + ``heads/patchfrog/migrate/acme-ai/abc123``) currently points at, + or ``None`` if it does not exist -- never raises on a missing + ref, since "does this migration branch already exist" is a + routine, expected question (M9.2/M9.6 idempotency), not an + error.""" + + try: + data = await self._get_json(installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/ref/{ref}") + except GitHubNotFoundError: + return None + try: + sha = data["object"]["sha"] + except (KeyError, TypeError) as exc: + raise GitHubResponseError("Malformed git ref response from GitHub") from exc + if not isinstance(sha, str): + raise GitHubResponseError("Malformed git ref response from GitHub") + return sha + + async def create_ref(self, *, installation_id: int, owner: str, repository: str, ref: str, sha: str) -> None: + """Create a new ref (``ref`` fully qualified, e.g. + ``refs/heads/patchfrog/migrate/acme-ai/abc123``) pointing at + ``sha``. Raises :class:`patchfrog.github.errors.GitHubUnprocessableError` + if the ref already exists -- callers that expect a possible + collision (re-running against an already-created branch) must + catch this and fall back to :meth:`update_ref` themselves; this + method never silently overwrites.""" + + await self._post_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/refs", + json_body={"ref": ref, "sha": sha}, + ) + + async def update_ref( + self, *, installation_id: int, owner: str, repository: str, ref: str, sha: str, force: bool = False + ) -> None: + """Move an existing ref (``ref`` without the ``refs/`` prefix, + e.g. ``heads/patchfrog/migrate/acme-ai/abc123``) to ``sha``. + ``force=False`` (the default) only ever performs a fast-forward; + GitHub rejects a non-fast-forward move with 422 unless + ``force=True`` is explicitly passed -- a migration branch is only + ever force-moved when PatchFrog itself owns every commit on it + (see :mod:`patchfrog.migration_pr.publisher`), never a branch a + human may have pushed additional commits to.""" + + await self._patch_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/refs/{ref}", + json_body={"sha": sha, "force": force}, + ) + + async def get_commit_tree_sha(self, *, installation_id: int, owner: str, repository: str, commit_sha: str) -> str: + data = await self._get_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/commits/{commit_sha}" + ) + try: + sha = data["tree"]["sha"] + except (KeyError, TypeError) as exc: + raise GitHubResponseError("Malformed git commit response from GitHub") from exc + if not isinstance(sha, str): + raise GitHubResponseError("Malformed git commit response from GitHub") + return sha + + async def create_tree( + self, *, installation_id: int, owner: str, repository: str, base_tree_sha: str, entries: list[GitTreeEntry] + ) -> str: + """Create a new tree layering ``entries`` (full file content, not + a diff) on top of ``base_tree_sha``. GitHub creates the backing + blobs server-side from ``content`` -- no separate blob-creation + call. Returns the new tree's SHA.""" + + payload = { + "base_tree": base_tree_sha, + "tree": [{"path": e.path, "mode": e.mode, "type": "blob", "content": e.content} for e in entries], + } + data = await self._post_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/trees", json_body=payload, + ) + try: + sha = data["sha"] + except (KeyError, TypeError) as exc: + raise GitHubResponseError("Malformed git tree response from GitHub") from exc + if not isinstance(sha, str): + raise GitHubResponseError("Malformed git tree response from GitHub") + return sha + + async def create_commit( + self, *, installation_id: int, owner: str, repository: str, message: str, tree_sha: str, parent_sha: str + ) -> str: + payload = {"message": message, "tree": tree_sha, "parents": [parent_sha]} + data = await self._post_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/git/commits", json_body=payload, + ) + try: + sha = data["sha"] + except (KeyError, TypeError) as exc: + raise GitHubResponseError("Malformed git commit-creation response from GitHub") from exc + if not isinstance(sha, str): + raise GitHubResponseError("Malformed git commit-creation response from GitHub") + return sha + + async def create_pull_request( + self, *, installation_id: int, owner: str, repository: str, title: str, body: str, head: str, base: str + ) -> PullRequestMetadata: + payload = {"title": title, "body": body, "head": head, "base": base} + data = await self._post_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/pulls", json_body=payload, + ) + return _parse_pull_request(data) + + async def list_pull_requests( + self, *, installation_id: int, owner: str, repository: str, head: str | None = None, state: str = "all" + ) -> list[PullRequestMetadata]: + """``head``, when given, must be ``":"`` (GitHub's + own required format) -- used to find any existing PR from + PatchFrog's own migration branch (M9.6 idempotency), never a + substring/heuristic match.""" + + params: dict[str, Any] = {"state": state, "per_page": _FILES_PER_PAGE} + if head is not None: + params["head"] = head + pull_requests: list[PullRequestMetadata] = [] + for page in range(1, _MAX_FILES_PAGES + 1): + data = await self._get_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/pulls", + params={**params, "page": page}, + ) + if not isinstance(data, list): + raise GitHubResponseError(f"Expected a list of pull requests, got {type(data).__name__}") + pull_requests.extend(_parse_pull_request(item) for item in data) + if len(data) < _FILES_PER_PAGE: + break + return pull_requests + + async def update_pull_request( + self, + *, + installation_id: int, + owner: str, + repository: str, + number: int, + title: str | None = None, + body: str | None = None, + ) -> PullRequestMetadata: + payload: dict[str, Any] = {} + if title is not None: + payload["title"] = title + if body is not None: + payload["body"] = body + data = await self._patch_json( + installation_id=installation_id, path=f"/repos/{owner}/{repository}/pulls/{number}", json_body=payload, + ) + return _parse_pull_request(data) + async def list_pull_request_files( self, *, installation_id: int, ref: PullRequestRef ) -> list[ChangedFile]: @@ -218,13 +373,13 @@ async def create_pull_request_review( return _parse_submitted_review(data) async def list_check_runs( - self, *, installation_id: int, ref: PullRequestRef, head_sha: str + self, *, installation_id: int, ref: PullRequestRef, head_sha: str, check_name: str = "PatchFrog review" ) -> list[GitHubCheckRun]: path = f"/repos/{ref.owner}/{ref.repository}/commits/{head_sha}/check-runs" data = await self._get_json( installation_id=installation_id, path=path, - params={"check_name": "PatchFrog review", "filter": "all", "per_page": 100}, + params={"check_name": check_name, "filter": "all", "per_page": 100}, ) if not isinstance(data, dict) or not isinstance(data.get("check_runs"), list): raise GitHubResponseError("Malformed check-runs response from GitHub") diff --git a/patchfrog/migration_pr/__init__.py b/patchfrog/migration_pr/__init__.py new file mode 100644 index 0000000..cc963e8 --- /dev/null +++ b/patchfrog/migration_pr/__init__.py @@ -0,0 +1,35 @@ +"""Evidence-Backed Automated Migration Pull Requests -- M9. + +Given a sufficiently verified migration (M8), decides whether PatchFrog +may open a GitHub PR at all, builds a deterministic branch/commit/PR plan +bound to the exact evidence it was verified against, and publishes it +through the existing GitHub Check infrastructure -- reusing +:mod:`patchfrog.publishing.checks`'s Protocol, never a second publishing +system. No LLM ever decides eligibility or content; no automatic merge. +See ``docs/migration-pr.md``. +""" + +from patchfrog.migration_pr.domain import ( + MIGRATION_PR_VERSION, + MigrationPREligibility, + MigrationPRPlan, + MigrationPRPolicy, + MigrationPRStatus, + MigrationPullRequest, +) +from patchfrog.migration_pr.integrity import MigrationPRIntegrityError +from patchfrog.migration_pr.planner import build_pr_plan +from patchfrog.migration_pr.publisher import MigrationPRPublicationMode, MigrationPRPublisher + +__all__ = [ + "MIGRATION_PR_VERSION", + "MigrationPREligibility", + "MigrationPRIntegrityError", + "MigrationPRPlan", + "MigrationPRPolicy", + "MigrationPRPublicationMode", + "MigrationPRPublisher", + "MigrationPRStatus", + "MigrationPullRequest", + "build_pr_plan", +] diff --git a/patchfrog/migration_pr/branch.py b/patchfrog/migration_pr/branch.py new file mode 100644 index 0000000..1ff71c6 --- /dev/null +++ b/patchfrog/migration_pr/branch.py @@ -0,0 +1,38 @@ +"""Deterministic migration branch naming (M9.2). + + patchfrog/migrate// + +Stable for the same migration identity (same provider, same upstream +change fingerprint) -- a re-run reconciles the same branch rather than +creating a new one (M9.6). ``provider`` is sanitized to a safe, git-ref- +legal token; nothing here is ever interpolated into a shell command (the +branch is only ever used as a JSON field in a GitHub API request body -- +see :mod:`patchfrog.github.client`), so there is no command-injection +surface, only a git-ref-syntax one, which this still guards defensively. +""" + +from __future__ import annotations + +import re + +_UNSAFE_REF_CHARS = re.compile(r"[^a-z0-9._-]+") +_SHORT_ID_LENGTH = 12 + + +def _sanitize(token: str) -> str: + safe = _UNSAFE_REF_CHARS.sub("-", token.lower()).strip("-.") + return safe or "unknown" + + +def migration_branch_name(*, provider: str | None, change_fingerprint: str) -> str: + """``provider`` may be ``None`` (no explicit provider identity was + ever supplied for this upstream change) -- falls back to a stable + ``"dependency"`` token rather than omitting the segment, so the + branch shape is always exactly three path segments.""" + + safe_provider = _sanitize(provider) if provider else "dependency" + short_id = change_fingerprint[:_SHORT_ID_LENGTH] or "0" * _SHORT_ID_LENGTH + return f"patchfrog/migrate/{safe_provider}/{short_id}" + + +__all__ = ["migration_branch_name"] diff --git a/patchfrog/migration_pr/check.py b/patchfrog/migration_pr/check.py new file mode 100644 index 0000000..9080aec --- /dev/null +++ b/patchfrog/migration_pr/check.py @@ -0,0 +1,86 @@ +"""Idempotent GitHub Check Run lifecycle for one migration branch head +(M9.5) -- reuses the existing :class:`patchfrog.publishing.checks.CheckRunClient` +Protocol and :class:`GitHubClient` verbatim; this is a second, parallel +check *name*/external-id scheme, never a second check-run publishing +mechanism. +""" + +from __future__ import annotations + +from patchfrog.domain.github_check import ( + GitHubCheckConclusion, + GitHubCheckOutput, + GitHubCheckRun, + GitHubCheckRunInput, + GitHubCheckStatus, +) +from patchfrog.domain.pull_request import PullRequestRef +from patchfrog.migration_pr.domain import MigrationPREligibility +from patchfrog.migration_verification.domain import VerificationOutcome +from patchfrog.publishing.checks import CheckRunClient + +CHECK_NAME = "PatchFrog Migration Verification" + + +def build_external_id(*, owner: str, repository: str, change_fingerprint: str, patch_fingerprint: str) -> str: + return f"patchfrog-migration:{owner}/{repository}:{change_fingerprint}:{patch_fingerprint}" + + +def build_check_input( + *, + head_sha: str, + external_id: str, + outcome: VerificationOutcome | None, + eligibility: MigrationPREligibility | None, + title: str, + summary: str, + details_url: str | None = None, +) -> GitHubCheckRunInput: + status, conclusion = _presentation(outcome, eligibility) + return GitHubCheckRunInput( + name=CHECK_NAME, head_sha=head_sha, external_id=external_id, status=status, conclusion=conclusion, + output=GitHubCheckOutput(title=title, summary=summary), details_url=details_url, + ) + + +def _presentation( + outcome: VerificationOutcome | None, eligibility: MigrationPREligibility | None +) -> tuple[GitHubCheckStatus, GitHubCheckConclusion | None]: + if outcome is None: + return GitHubCheckStatus.IN_PROGRESS, None + if outcome is VerificationOutcome.VERIFIED: + return GitHubCheckStatus.COMPLETED, GitHubCheckConclusion.SUCCESS + if outcome is VerificationOutcome.PARTIALLY_VERIFIED: + conclusion = ( + GitHubCheckConclusion.NEUTRAL if eligibility is MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL + else GitHubCheckConclusion.ACTION_REQUIRED + ) + return GitHubCheckStatus.COMPLETED, conclusion + if outcome is VerificationOutcome.HUMAN_REQUIRED: + return GitHubCheckStatus.COMPLETED, GitHubCheckConclusion.NEUTRAL + if outcome is VerificationOutcome.REGRESSION_DETECTED: + return GitHubCheckStatus.COMPLETED, GitHubCheckConclusion.FAILURE + if outcome is VerificationOutcome.FAILED: + return GitHubCheckStatus.COMPLETED, GitHubCheckConclusion.FAILURE + return GitHubCheckStatus.COMPLETED, GitHubCheckConclusion.NEUTRAL # UNVERIFIED + + +class MigrationCheckPublisher: + def __init__(self, *, client: CheckRunClient, installation_id: int) -> None: + self._client = client + self._installation_id = installation_id + + async def reconcile( + self, *, owner: str, repository: str, head_sha: str, check: GitHubCheckRunInput + ) -> GitHubCheckRun: + ref = PullRequestRef(owner=owner, repository=repository, number=0) + existing = await self._client.list_check_runs(installation_id=self._installation_id, ref=ref, head_sha=head_sha) + match = next((item for item in existing if item.name == CHECK_NAME and item.external_id == check.external_id), None) + if match is None: + return await self._client.create_check_run(installation_id=self._installation_id, ref=ref, check=check) + return await self._client.update_check_run( + installation_id=self._installation_id, ref=ref, check_run_id=match.id, check=check, + ) + + +__all__ = ["CHECK_NAME", "MigrationCheckPublisher", "build_check_input", "build_external_id"] diff --git a/patchfrog/migration_pr/domain.py b/patchfrog/migration_pr/domain.py new file mode 100644 index 0000000..6c08453 --- /dev/null +++ b/patchfrog/migration_pr/domain.py @@ -0,0 +1,150 @@ +"""Pure domain model for Evidence-Backed Automated Migration PRs -- M9. + +A :class:`MigrationPRPlan` is the complete, deterministic output of +deciding whether/how to publish a migration -- everything a dry-run needs +to print, and everything a real publish needs to write, with no further +decisions left to make at write time (mirrors +:class:`patchfrog.publishing.domain.ReviewPublicationPlan`'s own role for +review publishing). + +No LLM ever decides eligibility, branch identity, or PR content here -- +every field is derived from already-computed M6/M7/M8 evidence. +""" + +from __future__ import annotations + +import hashlib +import json +from dataclasses import dataclass, field +from enum import StrEnum + +#: Bumped whenever branch-naming, eligibility-policy, dossier-rendering, +#: or idempotency-identity logic changes materially enough that a prior +#: MigrationPRPlan/MigrationPullRequest can no longer be considered +#: equivalent to what re-running now would produce. +MIGRATION_PR_VERSION = 1 + + +class MigrationPREligibility(StrEnum): + """M9.3's publication eligibility decision -- a pure function of a + :class:`patchfrog.migration_verification.domain.VerificationOutcome` + and operator policy. Never silently lowers the bar: a + ``PARTIALLY_VERIFIED`` migration only ever reaches + ``OPEN_WITH_OPERATOR_APPROVAL`` when the operator has explicitly opted + in, and the resulting PR must be visibly marked partial (M9.3).""" + + #: VERIFIED -- may auto-open. + AUTO_OPEN = "auto_open" + #: PARTIALLY_VERIFIED and the operator's policy allows it -- must be + #: visibly marked in the dossier (see :mod:`patchfrog.migration_pr.dossier`). + OPEN_WITH_OPERATOR_APPROVAL = "open_with_operator_approval" + #: HUMAN_REQUIRED -- a plan/report may still be produced (dry-run), + #: but never a code PR by default. + PLAN_ONLY = "plan_only" + #: UNVERIFIED / FAILED / REGRESSION_DETECTED, or PARTIALLY_VERIFIED + #: without operator opt-in -- no PR, no plan-only report either. + NOT_ELIGIBLE = "not_eligible" + + +class MigrationPRStatus(StrEnum): + """Terminal (or in-flight) disposition of one migration PR publication + attempt -- mirrors :class:`patchfrog.publishing.domain.ReviewPublicationStatus`'s + own shape.""" + + PLANNED = "planned" + DRY_RUN = "dry_run" + PUBLISHING = "publishing" + OPENED = "opened" + UPDATED = "updated" + NO_OP_UNCHANGED = "no_op_unchanged" + SKIPPED_NOT_ELIGIBLE = "skipped_not_eligible" + STALE_REQUIRES_REGENERATION = "stale_requires_regeneration" + FAILED = "failed" + + +@dataclass(frozen=True, slots=True) +class MigrationPRPolicy: + """Operator-controlled publication policy (M9.3) -- never hardcoded, + never silently relaxed by this package itself.""" + + #: Whether a PARTIALLY_VERIFIED migration may open a (visibly marked) + #: PR at all. Off by default -- PatchFrog never lowers the bar on its + #: own. + allow_partially_verified: bool = False + + +@dataclass(frozen=True, slots=True) +class MigrationPRLinkage: + """Everything M9 idempotency, staleness, and integrity checking key + on -- the migration-PR-specific analogue of + :class:`patchfrog.migration.domain.PatchLinkage`.""" + + change_fingerprint: str + dependency_key: str | None + repository: str + base_commit_sha: str + plan_fingerprint: str + patch_fingerprint: str + verification_plan_fingerprint: str + bundle_fingerprint: str + engine_version: int = MIGRATION_PR_VERSION + + def identity_key(self) -> str: + """The stable identity a duplicate-PR check keys on: same + repository + same upstream change + same base/engine version -- + deliberately independent of the patch/verification fingerprints, + so a *legitimately regenerated* patch for the same upstream + change updates the existing PR (M9.6) rather than opening a + second one.""" + + payload = { + "repository": self.repository, "change_fingerprint": self.change_fingerprint, + "engine_version": self.engine_version, + } + return hashlib.sha256(json.dumps(payload, sort_keys=True).encode()).hexdigest() + + +@dataclass(frozen=True, slots=True) +class MigrationPRPlan: + """The complete, deterministic output of the publication planner.""" + + linkage: MigrationPRLinkage + eligibility: MigrationPREligibility + eligibility_reason: str + branch_name: str + commit_message: str + pr_title: str + pr_body: str + check_title: str + check_summary: str + + @property + def may_open_code_pr(self) -> bool: + return self.eligibility in ( + MigrationPREligibility.AUTO_OPEN, MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL, + ) + + +@dataclass(frozen=True, slots=True) +class MigrationPullRequest: + """One durable migration-PR publication record.""" + + id: str | None + number: int | None + html_url: str | None + status: MigrationPRStatus + linkage: MigrationPRLinkage + branch_name: str + reason: str | None = None + errors: tuple[str, ...] = field(default_factory=tuple) + + +__all__ = [ + "MIGRATION_PR_VERSION", + "MigrationPREligibility", + "MigrationPRLinkage", + "MigrationPRPlan", + "MigrationPRPolicy", + "MigrationPRStatus", + "MigrationPullRequest", +] diff --git a/patchfrog/migration_pr/dossier.py b/patchfrog/migration_pr/dossier.py new file mode 100644 index 0000000..40be865 --- /dev/null +++ b/patchfrog/migration_pr/dossier.py @@ -0,0 +1,156 @@ +"""PR body / Change Dossier rendering (M9.4). + +A concise, evidence-backed summary -- never raw logs, never AI prose. +Every section is built directly from already-computed M6/M7/M8 evidence +(:class:`~patchfrog.upstream.domain.ExternalChangeEvent`, +:class:`~patchfrog.upstream.blast_radius.BlastRadius`, +:class:`~patchfrog.migration.domain.MigrationPlan`/``GeneratedPatch``, +:class:`~patchfrog.migration_verification.domain.MigrationEvidenceBundle`). +""" + +from __future__ import annotations + +from patchfrog.migration.domain import GeneratedPatch, MigrationPlan +from patchfrog.migration_pr.domain import MigrationPREligibility, MigrationPRLinkage +from patchfrog.migration_pr.marker import render_marker, sanitize_untrusted_text +from patchfrog.migration_verification.domain import ( + CheckStatus, + MigrationEvidenceBundle, + VerificationStepKind, +) +from patchfrog.upstream.blast_radius import BlastRadius +from patchfrog.upstream.domain import ExternalChangeEvent + + +def _dependency_label(event: ExternalChangeEvent) -> str: + target = event.target + parts = [p for p in (target.provider_key, target.package_name, target.dependency_key) if p] + return parts[0] if parts else "unknown dependency" + + +def render_pr_title(event: ExternalChangeEvent, *, repository: str) -> str: + label = _dependency_label(event) + old_version = event.old.version or "?" + new_version = event.new.version or "?" + return f"PatchFrog: migrate {label} {old_version} -> {new_version}" + + +def _upstream_section(event: ExternalChangeEvent) -> list[str]: + lines = [ + "## Upstream change", "", + f"- Dependency: `{_dependency_label(event)}`", + f"- Version: `{event.old.version or '?'}` -> `{event.new.version or '?'}`", + f"- Compatibility classification: **{event.classification.risk.value.upper()}** " + f"({event.classification.compatibility.value})", + ] + if event.classification.reasons: + lines.append(f"- Reasons: {', '.join(sanitize_untrusted_text(r) for r in event.classification.reasons)}") + breaking = [item for item in event.diff if item.compatibility.value == "breaking"][:10] + if breaking: + lines.append("") + lines.append("Breaking changes addressed by this migration:") + for item in breaking: + lines.append(f"- `{item.location}`: {sanitize_untrusted_text(item.explanation)}") + return lines + + +def _impact_section(blast_radii: tuple[BlastRadius, ...]) -> list[str]: + lines = ["", "## Impact", ""] + if not blast_radii: + lines.append("- No blast radius evidence was computed for this migration.") + return lines + for radius in blast_radii: + summary = radius.summary() + lines.append( + f"- `{radius.dependency_key}`: {summary['direct_sites']} direct usage site(s), " + f"{summary['direct']} direct symbol(s), {summary['transitive']} transitively affected, " + f"{summary['related_tests']} related test file(s) across {summary['modules']} module(s)" + ) + return lines + + +def _migration_section(plan: MigrationPlan, patch: GeneratedPatch | None) -> list[str]: + lines = ["", "## Migration", ""] + strategies = sorted({s.strategy.value for s in plan.automatic_steps}) + lines.append(f"- Strategies applied: {', '.join(strategies) if strategies else 'none (no automatic steps)'}") + if patch is not None: + lines.append(f"- Files changed: {', '.join(f'`{f}`' for f in patch.modified_files) or 'none'}") + if plan.unresolved_steps: + lines.append(f"- Human-required steps ({len(plan.unresolved_steps)}):") + for step in plan.unresolved_steps[:10]: + lines.append(f" - `{step.target.location}`: {sanitize_untrusted_text(step.residual_uncertainty)}") + return lines + + +def _verification_section(bundle: MigrationEvidenceBundle) -> list[str]: + lines = ["", "## Verification", "", f"- Final outcome: **{bundle.outcome.value.upper()}**"] + for reason in bundle.outcome_reasons: + lines.append(f" - {sanitize_untrusted_text(reason)}") + + test_steps = [e for e in bundle.step_evidence if e.kind in ( + VerificationStepKind.TARGETED_UNIT_TEST, VerificationStepKind.TARGETED_INTEGRATION_TEST + )] + if test_steps: + passed = sum(1 for e in test_steps if e.status is CheckStatus.PASSED) + lines.append(f"- Targeted tests: {passed}/{len(test_steps)} passed") + + if bundle.contract_checks: + passed_contract = sum(1 for c in bundle.contract_checks if c.status is CheckStatus.PASSED) + lines.append(f"- Contract checks: {passed_contract}/{len(bundle.contract_checks)} passed") + + type_checks = [e for e in bundle.step_evidence if e.kind is VerificationStepKind.TYPE_CHECK] + if type_checks: + lines.append(f"- Type check: {type_checks[0].status.value}") + + if bundle.baseline_comparisons: + lines.append("- Baseline vs. patched evidence:") + for comparison in bundle.baseline_comparisons: + lines.append(f" - {comparison.baseline_status.value} -> {comparison.patched_status.value} " + f"({comparison.outcome.value})") + lines.append(f"- Evidence strength: {bundle.evidence_strength.value}") + return lines + + +def render_pr_body( + *, + event: ExternalChangeEvent, + plan: MigrationPlan, + patch: GeneratedPatch | None, + bundle: MigrationEvidenceBundle, + blast_radii: tuple[BlastRadius, ...], + linkage: MigrationPRLinkage, + eligibility: MigrationPREligibility, +) -> str: + sections: list[str] = [] + if eligibility is MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL: + sections.append( + "> **PARTIALLY VERIFIED** -- this migration was opened under an explicit operator policy that " + "allows publishing partially-verified migrations. Review the Verification section below carefully " + "before merging.\n" + ) + sections.extend(_upstream_section(event)) + sections.extend(_impact_section(blast_radii)) + sections.extend(_migration_section(plan, patch)) + sections.extend(_verification_section(bundle)) + sections.extend([ + "", "## Residual risk", "", f"- **{bundle.residual_risk.value.upper()}**", + "", "## Evidence identity", "", + f"- Upstream change fingerprint: `{linkage.change_fingerprint}`", + f"- Patch fingerprint: `{linkage.patch_fingerprint}`", + f"- Verification plan fingerprint: `{linkage.verification_plan_fingerprint}`", + f"- Evidence bundle fingerprint: `{bundle.bundle_fingerprint}`", + f"- Repository base SHA: `{linkage.base_commit_sha}`", + "", + render_marker(linkage.identity_key()), + ]) + return "\n".join(sections) + "\n" + + +def render_check_summary(bundle: MigrationEvidenceBundle) -> tuple[str, str]: + title = f"Migration verification: {bundle.outcome.value.replace('_', ' ')}" + summary = f"PatchFrog {bundle.outcome.value.replace('_', ' ')} this migration (evidence strength: " \ + f"{bundle.evidence_strength.value}, residual risk: {bundle.residual_risk.value})." + return title, summary + + +__all__ = ["render_check_summary", "render_pr_body", "render_pr_title"] diff --git a/patchfrog/migration_pr/eligibility.py b/patchfrog/migration_pr/eligibility.py new file mode 100644 index 0000000..b444943 --- /dev/null +++ b/patchfrog/migration_pr/eligibility.py @@ -0,0 +1,47 @@ +"""Migration PR publication eligibility policy (M9.3). + +A pure function of a verification outcome and operator policy -- never +silently lowers the bar. See +:class:`patchfrog.migration_pr.domain.MigrationPREligibility` for exactly +what each value means and +:class:`patchfrog.migration_verification.domain.VerificationOutcome` for +the outcome vocabulary this maps from. +""" + +from __future__ import annotations + +from patchfrog.migration_pr.domain import MigrationPREligibility, MigrationPRPolicy +from patchfrog.migration_verification.domain import VerificationOutcome + + +def determine_eligibility( + outcome: VerificationOutcome, *, policy: MigrationPRPolicy +) -> tuple[MigrationPREligibility, str]: + if outcome is VerificationOutcome.VERIFIED: + return MigrationPREligibility.AUTO_OPEN, "migration verification outcome is VERIFIED" + + if outcome is VerificationOutcome.PARTIALLY_VERIFIED: + if policy.allow_partially_verified: + return ( + MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL, + "migration verification outcome is PARTIALLY_VERIFIED and operator policy allows publication " + "(the PR will be visibly marked partial)", + ) + return ( + MigrationPREligibility.NOT_ELIGIBLE, + "migration verification outcome is PARTIALLY_VERIFIED and operator policy does not allow " + "publishing a partially-verified migration (set allow_partially_verified=true to permit this)", + ) + + if outcome is VerificationOutcome.HUMAN_REQUIRED: + return ( + MigrationPREligibility.PLAN_ONLY, + "migration verification outcome is HUMAN_REQUIRED -- a plan/report may be produced, " + "never an automatic code PR", + ) + + # UNVERIFIED, FAILED, REGRESSION_DETECTED + return MigrationPREligibility.NOT_ELIGIBLE, f"migration verification outcome is {outcome.value.upper()}" + + +__all__ = ["determine_eligibility"] diff --git a/patchfrog/migration_pr/fake_github.py b/patchfrog/migration_pr/fake_github.py new file mode 100644 index 0000000..89bba9b --- /dev/null +++ b/patchfrog/migration_pr/fake_github.py @@ -0,0 +1,160 @@ +"""Deterministic, in-memory :class:`~patchfrog.migration_pr.github_publisher.MigrationGitHubPublisher` +for tests (M9.9). + +Mirrors :class:`patchfrog.publishing.fake_publisher.FakeReviewPublisher`'s +role exactly -- structural typing is what makes this a legitimate stand-in +rather than a mock of internal plumbing. Simulates a minimal in-memory +"GitHub": refs, commits (content only, no real git objects), and pull +requests, plus check runs. No real GitHub PR is ever opened by using +this.""" + +from __future__ import annotations + +import hashlib +import json +from dataclasses import dataclass + +from patchfrog.domain.github_check import GitHubCheckRun, GitHubCheckRunInput +from patchfrog.domain.github_git import GitTreeEntry +from patchfrog.domain.pull_request import PullRequestMetadata + + +def _fake_sha(*parts: str) -> str: + return hashlib.sha256("|".join(parts).encode()).hexdigest()[:40] + + +@dataclass +class _FakePullRequest: + number: int + title: str + body: str + head_ref: str + base_ref: str + head_sha: str + state: str = "open" + + def to_metadata(self, *, owner: str, repository: str) -> PullRequestMetadata: + return PullRequestMetadata( + number=self.number, title=self.title, body=self.body, author="patchfrog[bot]", + base_branch=self.base_ref, head_branch=self.head_ref, base_sha=self.head_sha, head_sha=self.head_sha, + html_url=f"https://github.com/{owner}/{repository}/pull/{self.number}", state=self.state, + ) + + +class FakeMigrationGitHubPublisher: + """Not a subclass of anything -- satisfies + :class:`patchfrog.migration_pr.github_publisher.MigrationGitHubPublisher` + structurally.""" + + def __init__(self, *, refs: dict[str, str] | None = None) -> None: + #: "owner/repository:heads/" -> sha. + self._refs: dict[str, str] = dict(refs or {}) + #: sha -> tree sha (fake: just a digest over the entries). + self._commit_trees: dict[str, str] = {} + self._pull_requests: list[_FakePullRequest] = [] + self._next_pr_number = 100 + self.checks: list[GitHubCheckRun] = [] + self.create_ref_calls: list[str] = [] + self.update_ref_calls: list[str] = [] + self.create_pull_request_calls: list[str] = [] + self.update_pull_request_calls: list[int] = [] + + def set_ref(self, *, owner: str, repository: str, ref: str, sha: str) -> None: + self._refs[f"{owner}/{repository}:{ref}"] = sha + + async def get_ref(self, *, owner: str, repository: str, ref: str) -> str | None: + return self._refs.get(f"{owner}/{repository}:{ref}") + + async def create_ref(self, *, owner: str, repository: str, ref: str, sha: str) -> None: + key = f"{owner}/{repository}:{ref.removeprefix('refs/')}" + if key in self._refs: + from patchfrog.github.errors import GitHubUnprocessableError + + raise GitHubUnprocessableError(f"ref already exists: {ref}") + self._refs[key] = sha + self.create_ref_calls.append(ref) + + async def update_ref(self, *, owner: str, repository: str, ref: str, sha: str, force: bool = False) -> None: + self._refs[f"{owner}/{repository}:{ref}"] = sha + self.update_ref_calls.append(ref) + + async def get_commit_tree_sha(self, *, owner: str, repository: str, commit_sha: str) -> str: + return self._commit_trees.get(commit_sha, _fake_sha("tree", commit_sha)) + + async def create_tree( + self, *, owner: str, repository: str, base_tree_sha: str, entries: list[GitTreeEntry] + ) -> str: + payload = json.dumps( + [{"path": e.path, "content": e.content} for e in entries], sort_keys=True, + ) + return _fake_sha("tree", base_tree_sha, payload) + + async def create_commit( + self, *, owner: str, repository: str, message: str, tree_sha: str, parent_sha: str + ) -> str: + commit_sha = _fake_sha("commit", message, tree_sha, parent_sha) + self._commit_trees[commit_sha] = tree_sha + return commit_sha + + async def create_pull_request( + self, *, owner: str, repository: str, title: str, body: str, head: str, base: str + ) -> PullRequestMetadata: + self.create_pull_request_calls.append(head) + head_sha = self._refs.get(f"{owner}/{repository}:heads/{head}", "") + pr = _FakePullRequest( + number=self._next_pr_number, title=title, body=body, head_ref=head, base_ref=base, head_sha=head_sha, + ) + self._next_pr_number += 1 + self._pull_requests.append(pr) + return pr.to_metadata(owner=owner, repository=repository) + + async def list_pull_requests( + self, *, owner: str, repository: str, head: str | None = None, state: str = "all" + ) -> list[PullRequestMetadata]: + results = [] + for pr in self._pull_requests: + if head is not None and f"{owner}:{pr.head_ref}" != head: + continue + if state != "all" and pr.state != state: + continue + results.append(pr.to_metadata(owner=owner, repository=repository)) + return results + + async def update_pull_request( + self, *, owner: str, repository: str, number: int, title: str | None = None, body: str | None = None + ) -> PullRequestMetadata: + self.update_pull_request_calls.append(number) + pr = next(p for p in self._pull_requests if p.number == number) + if title is not None: + pr.title = title + if body is not None: + pr.body = body + pr.head_sha = self._refs.get(f"{owner}/{repository}:heads/{pr.head_ref}", pr.head_sha) + return pr.to_metadata(owner=owner, repository=repository) + + def close_pull_request(self, number: int) -> None: + next(p for p in self._pull_requests if p.number == number).state = "closed" + + async def reconcile_check( + self, *, owner: str, repository: str, head_sha: str, check: GitHubCheckRunInput + ) -> GitHubCheckRun: + match = next( + (c for c in self.checks if c.external_id == check.external_id and c.head_sha == head_sha), None + ) + if match is None: + run = GitHubCheckRun( + id=len(self.checks) + 1, name=check.name, head_sha=head_sha, external_id=check.external_id, + status=check.status, conclusion=check.conclusion, + ) + self.checks.append(run) + return run + self.checks.remove(match) + updated = GitHubCheckRun( + id=match.id, name=check.name, head_sha=head_sha, external_id=check.external_id, status=check.status, + conclusion=check.conclusion, + ) + self.checks.append(updated) + return updated + + +__all__ = ["FakeMigrationGitHubPublisher"] diff --git a/patchfrog/migration_pr/github_publisher.py b/patchfrog/migration_pr/github_publisher.py new file mode 100644 index 0000000..862207c --- /dev/null +++ b/patchfrog/migration_pr/github_publisher.py @@ -0,0 +1,130 @@ +"""Narrow GitHub boundary for migration PR publication (M9.9). + +Mirrors :mod:`patchfrog.publishing.github_publisher`'s role exactly: +structural typing via ``Protocol`` names only the operations +:mod:`patchfrog.migration_pr.publisher` actually needs, so a fake +satisfying this Protocol is a legitimate stand-in for tests -- never a +mock of internal plumbing, and no real GitHub PR is ever opened by an +automated test (M9.9's own requirement). +""" + +from __future__ import annotations + +from typing import Protocol + +from patchfrog.domain.github_check import GitHubCheckRun, GitHubCheckRunInput +from patchfrog.domain.github_git import GitTreeEntry +from patchfrog.domain.pull_request import PullRequestMetadata +from patchfrog.github.client import GitHubClient +from patchfrog.migration_pr.check import MigrationCheckPublisher + + +class MigrationGitHubPublisher(Protocol): + async def get_ref(self, *, owner: str, repository: str, ref: str) -> str | None: ... + + async def create_ref(self, *, owner: str, repository: str, ref: str, sha: str) -> None: ... + + async def update_ref(self, *, owner: str, repository: str, ref: str, sha: str, force: bool = False) -> None: ... + + async def get_commit_tree_sha(self, *, owner: str, repository: str, commit_sha: str) -> str: ... + + async def create_tree( + self, *, owner: str, repository: str, base_tree_sha: str, entries: list[GitTreeEntry] + ) -> str: ... + + async def create_commit( + self, *, owner: str, repository: str, message: str, tree_sha: str, parent_sha: str + ) -> str: ... + + async def create_pull_request( + self, *, owner: str, repository: str, title: str, body: str, head: str, base: str + ) -> PullRequestMetadata: ... + + async def list_pull_requests( + self, *, owner: str, repository: str, head: str | None = None, state: str = "all" + ) -> list[PullRequestMetadata]: ... + + async def update_pull_request( + self, *, owner: str, repository: str, number: int, title: str | None = None, body: str | None = None + ) -> PullRequestMetadata: ... + + async def reconcile_check( + self, *, owner: str, repository: str, head_sha: str, check: GitHubCheckRunInput + ) -> GitHubCheckRun: ... + + +class GitHubClientMigrationPublisher: + """The real :class:`MigrationGitHubPublisher` -- a thin, + installation-scoped wrapper around :class:`GitHubClient`, exactly the + role :class:`patchfrog.publishing.github_publisher.GitHubClientReviewPublisher` + plays for review publishing.""" + + def __init__(self, *, github_client: GitHubClient, installation_id: int) -> None: + self._client = github_client + self._installation_id = installation_id + self._check_publisher = MigrationCheckPublisher(client=github_client, installation_id=installation_id) + + async def get_ref(self, *, owner: str, repository: str, ref: str) -> str | None: + return await self._client.get_ref(installation_id=self._installation_id, owner=owner, repository=repository, ref=ref) + + async def create_ref(self, *, owner: str, repository: str, ref: str, sha: str) -> None: + await self._client.create_ref( + installation_id=self._installation_id, owner=owner, repository=repository, ref=ref, sha=sha, + ) + + async def update_ref(self, *, owner: str, repository: str, ref: str, sha: str, force: bool = False) -> None: + await self._client.update_ref( + installation_id=self._installation_id, owner=owner, repository=repository, ref=ref, sha=sha, force=force, + ) + + async def get_commit_tree_sha(self, *, owner: str, repository: str, commit_sha: str) -> str: + return await self._client.get_commit_tree_sha( + installation_id=self._installation_id, owner=owner, repository=repository, commit_sha=commit_sha, + ) + + async def create_tree( + self, *, owner: str, repository: str, base_tree_sha: str, entries: list[GitTreeEntry] + ) -> str: + return await self._client.create_tree( + installation_id=self._installation_id, owner=owner, repository=repository, base_tree_sha=base_tree_sha, + entries=entries, + ) + + async def create_commit( + self, *, owner: str, repository: str, message: str, tree_sha: str, parent_sha: str + ) -> str: + return await self._client.create_commit( + installation_id=self._installation_id, owner=owner, repository=repository, message=message, + tree_sha=tree_sha, parent_sha=parent_sha, + ) + + async def create_pull_request( + self, *, owner: str, repository: str, title: str, body: str, head: str, base: str + ) -> PullRequestMetadata: + return await self._client.create_pull_request( + installation_id=self._installation_id, owner=owner, repository=repository, title=title, body=body, + head=head, base=base, + ) + + async def list_pull_requests( + self, *, owner: str, repository: str, head: str | None = None, state: str = "all" + ) -> list[PullRequestMetadata]: + return await self._client.list_pull_requests( + installation_id=self._installation_id, owner=owner, repository=repository, head=head, state=state, + ) + + async def update_pull_request( + self, *, owner: str, repository: str, number: int, title: str | None = None, body: str | None = None + ) -> PullRequestMetadata: + return await self._client.update_pull_request( + installation_id=self._installation_id, owner=owner, repository=repository, number=number, title=title, + body=body, + ) + + async def reconcile_check( + self, *, owner: str, repository: str, head_sha: str, check: GitHubCheckRunInput + ) -> GitHubCheckRun: + return await self._check_publisher.reconcile(owner=owner, repository=repository, head_sha=head_sha, check=check) + + +__all__ = ["GitHubClientMigrationPublisher", "MigrationGitHubPublisher"] diff --git a/patchfrog/migration_pr/integrity.py b/patchfrog/migration_pr/integrity.py new file mode 100644 index 0000000..6ca62c8 --- /dev/null +++ b/patchfrog/migration_pr/integrity.py @@ -0,0 +1,55 @@ +"""Evidence/patch integrity check (M9.8). + +Before a :class:`~patchfrog.migration_pr.domain.MigrationPRPlan` is even +built, verify that the evidence bundle a caller is about to publish from +actually corresponds to the exact upstream change, patch, and repository +state it claims to. A :class:`~patchfrog.migration_verification.domain.MigrationEvidenceBundle` +computed for one patch must never be silently treated as evidence for a +different one -- this is a programming-error guard (a caller passing +mismatched arguments), not a normal outcome, so it raises rather than +degrading to a weaker status. +""" + +from __future__ import annotations + +from patchfrog.migration.domain import GeneratedPatch +from patchfrog.migration_verification.domain import MigrationEvidenceBundle +from patchfrog.upstream.domain import ExternalChangeEvent + + +class MigrationPRIntegrityError(Exception): + """Raised when an evidence bundle does not correspond to the exact + upstream change / patch / repository state a migration PR plan is + about to be built from.""" + + +def verify_evidence_integrity( + *, + event: ExternalChangeEvent, + patch: GeneratedPatch | None, + bundle: MigrationEvidenceBundle, + base_commit_sha: str, +) -> None: + if bundle.change_fingerprint != event.fingerprint: + raise MigrationPRIntegrityError( + f"evidence bundle was computed for upstream change {bundle.change_fingerprint!r}, " + f"not the requested change {event.fingerprint!r}" + ) + + expected_patch_fingerprint = patch.fingerprint if patch is not None else "" + if bundle.patch_fingerprint != expected_patch_fingerprint: + raise MigrationPRIntegrityError( + f"evidence bundle was computed for patch {bundle.patch_fingerprint!r}, " + f"not the patch about to be published ({expected_patch_fingerprint!r}) -- " + "a VERIFIED bundle for one patch must never be reused for another" + ) + + if bundle.repository_head_sha != base_commit_sha: + raise MigrationPRIntegrityError( + f"evidence bundle was computed against repository head {bundle.repository_head_sha!r}, " + f"not the base commit this migration PR would publish against ({base_commit_sha!r}) -- " + "regenerate the evidence rather than publishing stale evidence" + ) + + +__all__ = ["MigrationPRIntegrityError", "verify_evidence_integrity"] diff --git a/patchfrog/migration_pr/marker.py b/patchfrog/migration_pr/marker.py new file mode 100644 index 0000000..35e3f72 --- /dev/null +++ b/patchfrog/migration_pr/marker.py @@ -0,0 +1,50 @@ +"""PatchFrog's machine-readable migration-PR marker. + +Embedded once, at the end of a migration PR's body, as an HTML comment +(invisible in GitHub's rendered markdown): + + + +``identity-key`` is :meth:`patchfrog.migration_pr.domain.MigrationPRLinkage.identity_key` +-- deterministic (repository + upstream change fingerprint + engine +version), unlike :mod:`patchfrog.publishing.marker`'s random per-attempt +UUID, because migration-PR idempotency is itself about *reusing* the same +PR across regenerated patches for the same upstream change (M9.6), not +about disambiguating concurrent attempts at one already-known row. +""" + +from __future__ import annotations + +import re + +_MARKER_PREFIX = "patchfrog:migration:" +_MARKER_RE = re.compile(r"") +_MARKER_LOOKALIKE_RE = re.compile(r"", re.DOTALL) + + +def render_marker(identity_key: str) -> str: + return f"" + + +def find_marker(body: str | None) -> str | None: + """Extract the identity key from a PR body, if present and + well-formed. Never raises -- a body with no marker, or one PatchFrog + never wrote, is simply not a match.""" + + if not body: + return None + match = _MARKER_RE.search(body) + return match.group(1) if match else None + + +def sanitize_untrusted_text(text: str) -> str: + """Strip any marker-shaped sequence from evidence-derived text before + interpolation into a PR body -- defense in depth, mirroring + :func:`patchfrog.publishing.marker.sanitize_untrusted_text`, even + though dossier content here is deterministic evidence, not free-form + AI output.""" + + return _MARKER_LOOKALIKE_RE.sub("[redacted-marker-like-sequence]", text) + + +__all__ = ["find_marker", "render_marker", "sanitize_untrusted_text"] diff --git a/patchfrog/migration_pr/planner.py b/patchfrog/migration_pr/planner.py new file mode 100644 index 0000000..06baee6 --- /dev/null +++ b/patchfrog/migration_pr/planner.py @@ -0,0 +1,64 @@ +"""Builds the complete, deterministic :class:`MigrationPRPlan` (M9.1/M9.4) +from already-computed M6/M7/M8 evidence -- ties together branch naming +(M9.2), eligibility policy (M9.3), and dossier rendering (M9.4).""" + +from __future__ import annotations + +from patchfrog.migration.domain import GeneratedPatch, MigrationPlan +from patchfrog.migration_pr.branch import migration_branch_name +from patchfrog.migration_pr.domain import MigrationPRLinkage, MigrationPRPlan, MigrationPRPolicy +from patchfrog.migration_pr.dossier import render_check_summary, render_pr_body, render_pr_title +from patchfrog.migration_pr.eligibility import determine_eligibility +from patchfrog.migration_pr.integrity import verify_evidence_integrity +from patchfrog.migration_verification.domain import MigrationEvidenceBundle +from patchfrog.upstream.blast_radius import BlastRadius +from patchfrog.upstream.domain import ExternalChangeEvent + + +def build_pr_plan( + *, + event: ExternalChangeEvent, + plan: MigrationPlan, + patch: GeneratedPatch | None, + bundle: MigrationEvidenceBundle, + blast_radii: tuple[BlastRadius, ...], + repository: str, + base_commit_sha: str, + #: The persisted M7 plan fingerprint (see + #: :meth:`patchfrog.migration.store.MigrationStore.record_plan`), + #: when already known -- purely informational here (M9 idempotency + #: keys on ``change_fingerprint``/``engine_version``, never this), + #: kept only so the evidence-identity section of the dossier can cite + #: it. Empty when the caller has not persisted a plan. + plan_fingerprint: str = "", + policy: MigrationPRPolicy | None = None, +) -> MigrationPRPlan: + verify_evidence_integrity(event=event, patch=patch, bundle=bundle, base_commit_sha=base_commit_sha) + + policy = policy or MigrationPRPolicy() + eligibility, reason = determine_eligibility(bundle.outcome, policy=policy) + + linkage = MigrationPRLinkage( + change_fingerprint=event.fingerprint, dependency_key=event.target.dependency_key, repository=repository, + base_commit_sha=base_commit_sha, plan_fingerprint=plan_fingerprint, + patch_fingerprint=patch.fingerprint if patch is not None else "", + verification_plan_fingerprint=bundle.plan.fingerprint(), bundle_fingerprint=bundle.bundle_fingerprint, + ) + branch_name = migration_branch_name(provider=event.target.provider_key, change_fingerprint=event.fingerprint) + pr_title = render_pr_title(event, repository=repository) + pr_body = render_pr_body( + event=event, plan=plan, patch=patch, bundle=bundle, blast_radii=blast_radii, linkage=linkage, + eligibility=eligibility, + ) + check_title, check_summary = render_check_summary(bundle) + dependency_label = event.target.provider_key or event.target.package_name or event.target.dependency_key or "dependency" + commit_message = f"PatchFrog: migrate {dependency_label} ({event.old.version or '?'} -> {event.new.version or '?'})" + + return MigrationPRPlan( + linkage=linkage, eligibility=eligibility, eligibility_reason=reason, branch_name=branch_name, + commit_message=commit_message, pr_title=pr_title, pr_body=pr_body, check_title=check_title, + check_summary=check_summary, + ) + + +__all__ = ["build_pr_plan"] diff --git a/patchfrog/migration_pr/publisher.py b/patchfrog/migration_pr/publisher.py new file mode 100644 index 0000000..b590542 --- /dev/null +++ b/patchfrog/migration_pr/publisher.py @@ -0,0 +1,213 @@ +"""Orchestrates one migration PR publication attempt end to end (M9.6/M9.7/M9.8). + + MigrationPRPlan (already built by :mod:`patchfrog.migration_pr.planner`) + -> eligibility gate -> idempotency lookup (identity_key) + -> [DRY_RUN: stop here, no write] + -> stale-base check (M9.7) -> integrity check (M9.8) + -> branch create/update -> PR open/update (M9.6) -> check run + +Migration branches are entirely PatchFrog-owned: every publish rebuilds +the branch fresh from the *current* base commit (never a three-way merge, +never preserving a human's own commits on it) -- a branch is only ever +force-moved because nothing but PatchFrog's own generated content is +expected to live there. GitHub and PatchFrog's database cannot share one +transaction, exactly like :mod:`patchfrog.publishing.service` -- the +identity is locked and a durable row exists *before* any GitHub write is +attempted, mirroring that module's own discipline. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from enum import StrEnum + +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from patchfrog.domain.github_git import GitTreeEntry +from patchfrog.github.errors import GitHubError, GitHubUnprocessableError +from patchfrog.migration.domain import GeneratedPatch +from patchfrog.migration_pr.check import build_check_input, build_external_id +from patchfrog.migration_pr.domain import ( + MigrationPREligibility, + MigrationPRPlan, + MigrationPRStatus, + MigrationPullRequest, +) +from patchfrog.migration_pr.github_publisher import MigrationGitHubPublisher +from patchfrog.migration_pr.store import MigrationPRStore +from patchfrog.migration_verification.domain import VerificationOutcome + + +class MigrationPRPublicationMode(StrEnum): + DRY_RUN = "dry_run" + PUBLISH = "publish" + + +def _owner_repository(full_name: str) -> tuple[str, str]: + owner, _, repository = full_name.partition("/") + return owner, repository + + +def _result( + plan: MigrationPRPlan, *, status: MigrationPRStatus, number: int | None = None, html_url: str | None = None, + reason: str | None = None, errors: tuple[str, ...] = (), +) -> MigrationPullRequest: + return MigrationPullRequest( + id=None, number=number, html_url=html_url, status=status, linkage=plan.linkage, branch_name=plan.branch_name, + reason=reason, errors=errors, + ) + + +class MigrationPRPublisher: + def __init__( + self, + *, + session_factory: async_sessionmaker[AsyncSession], + publisher: MigrationGitHubPublisher, + installation_id: int, + ) -> None: + self._session_factory = session_factory + self._publisher = publisher + self._installation_id = installation_id + self._store = MigrationPRStore() + + async def publish( + self, *, plan: MigrationPRPlan, patch: GeneratedPatch | None, mode: MigrationPRPublicationMode, + base_branch: str = "main", + ) -> MigrationPullRequest: + identity_key = plan.linkage.identity_key() + + if plan.eligibility is MigrationPREligibility.NOT_ELIGIBLE: + return await self._finalize(plan, self._result_no_write(plan, MigrationPRStatus.SKIPPED_NOT_ELIGIBLE, plan.eligibility_reason)) + + if plan.eligibility is MigrationPREligibility.PLAN_ONLY: + # Never a code PR, regardless of mode (M9.3: "optionally a + # plan/report, not a code PR by default" -- no policy knob + # is offered to change this, unlike PARTIALLY_VERIFIED). + return await self._finalize(plan, self._result_no_write(plan, MigrationPRStatus.PLANNED, plan.eligibility_reason)) + + if patch is None: + return await self._finalize( + plan, self._result_no_write(plan, MigrationPRStatus.SKIPPED_NOT_ELIGIBLE, "no generated patch to publish") + ) + + if mode is MigrationPRPublicationMode.DRY_RUN: + return await self._finalize(plan, self._result_no_write(plan, MigrationPRStatus.DRY_RUN, "dry run: no GitHub write performed")) + + # -- real publish from here ----------------------------------------------------------- + owner, repository = _owner_repository(plan.linkage.repository) + base_branch_ref = f"heads/{base_branch}" + + try: + current_base_sha = await self._publisher.get_ref(owner=owner, repository=repository, ref=base_branch_ref) + except GitHubError as exc: + return await self._finalize(plan, self._result_no_write(plan, MigrationPRStatus.FAILED, f"could not read base branch: {exc}")) + + if current_base_sha is None: + return await self._finalize(plan, self._result_no_write(plan, MigrationPRStatus.FAILED, "base branch not found")) + + # M9.7: stale-base protection. Exact match required -- a moved + # base means the plan/patch/evidence were computed against a + # repository state that no longer exists; never publish a stale + # VERIFIED status against a different base (regeneration is + # required, never assumed compatible). + if current_base_sha != plan.linkage.base_commit_sha: + return await self._finalize( + plan, + self._result_no_write( + plan, MigrationPRStatus.STALE_REQUIRES_REGENERATION, + f"base branch moved from {plan.linkage.base_commit_sha!r} to {current_base_sha!r}; regenerate", + ), + ) + + try: + base_tree_sha = await self._publisher.get_commit_tree_sha( + owner=owner, repository=repository, commit_sha=current_base_sha, + ) + entries = [GitTreeEntry(path=path, content=content) for path, content in sorted(patch.new_contents.items())] + tree_sha = await self._publisher.create_tree( + owner=owner, repository=repository, base_tree_sha=base_tree_sha, entries=entries, + ) + commit_sha = await self._publisher.create_commit( + owner=owner, repository=repository, message=plan.commit_message, tree_sha=tree_sha, + parent_sha=current_base_sha, + ) + branch_ref = f"heads/{plan.branch_name}" + existing_branch_sha = await self._publisher.get_ref(owner=owner, repository=repository, ref=branch_ref) + if existing_branch_sha is None: + try: + await self._publisher.create_ref( + owner=owner, repository=repository, ref=f"refs/{branch_ref}", sha=commit_sha, + ) + except GitHubUnprocessableError: + # Lost a race with a concurrent publish -- reconcile + # onto whatever now exists rather than failing. + await self._publisher.update_ref( + owner=owner, repository=repository, ref=branch_ref, sha=commit_sha, force=True, + ) + elif existing_branch_sha == commit_sha: + pass # identical content already there -- nothing to move. + else: + await self._publisher.update_ref( + owner=owner, repository=repository, ref=branch_ref, sha=commit_sha, force=True, + ) + + head = f"{owner}:{plan.branch_name}" + existing_prs = await self._publisher.list_pull_requests(owner=owner, repository=repository, head=head) + open_pr = next((pr for pr in existing_prs if pr.state == "open"), None) + closed_pr = next((pr for pr in existing_prs if pr.state != "open"), None) + + if open_pr is not None: + updated = await self._publisher.update_pull_request( + owner=owner, repository=repository, number=open_pr.number, title=plan.pr_title, body=plan.pr_body, + ) + result = _result(plan, status=MigrationPRStatus.UPDATED, number=updated.number, html_url=updated.html_url) + elif closed_pr is not None: + result = _result( + plan, status=MigrationPRStatus.NO_OP_UNCHANGED, number=closed_pr.number, html_url=closed_pr.html_url, + reason="a PR for this migration already exists but was closed by a human; not reopened automatically", + ) + else: + created = await self._publisher.create_pull_request( + owner=owner, repository=repository, title=plan.pr_title, body=plan.pr_body, + head=plan.branch_name, base=base_branch, + ) + result = _result(plan, status=MigrationPRStatus.OPENED, number=created.number, html_url=created.html_url) + + check_head_sha = commit_sha + external_id = build_external_id( + owner=owner, repository=repository, change_fingerprint=plan.linkage.change_fingerprint, + patch_fingerprint=plan.linkage.patch_fingerprint, + ) + check_input = build_check_input( + head_sha=check_head_sha, external_id=external_id, outcome=_outcome_for_check(plan), + eligibility=plan.eligibility, title=plan.check_title, summary=plan.check_summary, + details_url=result.html_url, + ) + await self._publisher.reconcile_check(owner=owner, repository=repository, head_sha=check_head_sha, check=check_input) + except GitHubError as exc: + result = self._result_no_write(plan, MigrationPRStatus.FAILED, str(exc)) + + return await self._finalize(plan, result, identity_key=identity_key) + + def _result_no_write(self, plan: MigrationPRPlan, status: MigrationPRStatus, reason: str) -> MigrationPullRequest: + return _result(plan, status=status, reason=reason) + + async def _finalize( + self, plan: MigrationPRPlan, result: MigrationPullRequest, *, identity_key: str | None = None, + ) -> MigrationPullRequest: + async with self._session_factory() as session: + row, _created = await self._store.upsert(session, plan=plan, result=result, recorded_at=datetime.now(UTC)) + await session.commit() + return _result(plan, status=result.status, number=row.pr_number, html_url=row.pr_html_url, reason=result.reason, errors=result.errors) + + +def _outcome_for_check(plan: MigrationPRPlan) -> VerificationOutcome | None: + if plan.eligibility is MigrationPREligibility.AUTO_OPEN: + return VerificationOutcome.VERIFIED + if plan.eligibility is MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL: + return VerificationOutcome.PARTIALLY_VERIFIED + return None + + +__all__ = ["MigrationPRPublicationMode", "MigrationPRPublisher"] diff --git a/patchfrog/migration_pr/report.py b/patchfrog/migration_pr/report.py new file mode 100644 index 0000000..73e7661 --- /dev/null +++ b/patchfrog/migration_pr/report.py @@ -0,0 +1,63 @@ +"""Machine-readable (JSON) and human-readable views of a +:class:`~patchfrog.migration_pr.domain.MigrationPRPlan` -- backs +``patchfrog migrations publish --dry-run``'s ``--json`` and text output +(M9.10). Never touches GitHub; a pure rendering of an already-built plan. +""" + +from __future__ import annotations + +from typing import Any + +from patchfrog.migration_pr.domain import MigrationPRPlan + + +def linkage_to_dict(plan: MigrationPRPlan) -> dict[str, Any]: + linkage = plan.linkage + return { + "identity_key": linkage.identity_key(), + "change_fingerprint": linkage.change_fingerprint, + "dependency_key": linkage.dependency_key, + "repository": linkage.repository, + "base_commit_sha": linkage.base_commit_sha, + "plan_fingerprint": linkage.plan_fingerprint, + "patch_fingerprint": linkage.patch_fingerprint, + "verification_plan_fingerprint": linkage.verification_plan_fingerprint, + "bundle_fingerprint": linkage.bundle_fingerprint, + "engine_version": linkage.engine_version, + } + + +def pr_plan_to_dict(plan: MigrationPRPlan) -> dict[str, Any]: + return { + "linkage": linkage_to_dict(plan), + "eligibility": plan.eligibility.value, + "eligibility_reason": plan.eligibility_reason, + "may_open_code_pr": plan.may_open_code_pr, + "branch_name": plan.branch_name, + "commit_message": plan.commit_message, + "pr_title": plan.pr_title, + "pr_body": plan.pr_body, + "check_title": plan.check_title, + "check_summary": plan.check_summary, + } + + +def render_pr_plan_text(plan: MigrationPRPlan) -> str: + lines = [ + f"Eligibility: {plan.eligibility.value.upper()}", + f" - {plan.eligibility_reason}", + f"Would open a code PR: {'yes' if plan.may_open_code_pr else 'no'}", + f"Proposed branch: {plan.branch_name}", + f"Commit title: {plan.commit_message}", + f"PR title: {plan.pr_title}", + f"Check title: {plan.check_title}", + f"Check summary: {plan.check_summary}", + "", + "PR body:", + "--------", + plan.pr_body, + ] + return "\n".join(lines) + "\n" + + +__all__ = ["linkage_to_dict", "pr_plan_to_dict", "render_pr_plan_text"] diff --git a/patchfrog/migration_pr/store.py b/patchfrog/migration_pr/store.py new file mode 100644 index 0000000..46f6d02 --- /dev/null +++ b/patchfrog/migration_pr/store.py @@ -0,0 +1,102 @@ +"""Idempotent persistence for migration PR publication (M9.1/M9.6). + +Identity is :meth:`patchfrog.migration_pr.domain.MigrationPRLinkage.identity_key` +(repository + upstream change fingerprint + engine version) -- mirrors +:mod:`patchfrog.migration.store`'s advisory-lock/fingerprint-keyed-upsert +pattern, but keyed on the *migration's* identity rather than the patch's, +since the whole point is reconciling the same row across a regenerated +patch (M9.6), never creating a duplicate. +""" + +from __future__ import annotations + +import hashlib +from datetime import UTC, datetime + +from sqlalchemy import select, text +from sqlalchemy.ext.asyncio import AsyncSession + +from patchfrog.migration_pr.domain import ( + MigrationPRLinkage, + MigrationPRPlan, + MigrationPRStatus, + MigrationPullRequest, +) +from patchfrog.persistence.models.migration_pr import MigrationPRModel + + +async def _advisory_lock(session: AsyncSession, key: str) -> None: + if session.bind is None or session.bind.dialect.name != "postgresql": + return + digest = hashlib.sha256(key.encode()).digest()[:8] + await session.execute( + text("SELECT pg_advisory_xact_lock(:key)"), {"key": int.from_bytes(digest, "big") & 0x7FFFFFFFFFFFFFFF} + ) + + +class MigrationPRStore: + async def get(self, session: AsyncSession, *, identity_key: str) -> MigrationPRModel | None: + return ( + await session.execute(select(MigrationPRModel).where(MigrationPRModel.identity_key == identity_key)) + ).scalar_one_or_none() + + async def upsert( + self, + session: AsyncSession, + *, + plan: MigrationPRPlan, + result: MigrationPullRequest, + recorded_at: datetime | None = None, + ) -> tuple[MigrationPRModel, bool]: + """Returns (row, created). One row per identity -- a re-run for + the same upstream change always updates the same row (M9.6), + recording whatever the latest patch/evidence/PR state is.""" + + linkage = plan.linkage + identity_key = linkage.identity_key() + now = recorded_at or datetime.now(UTC) + await _advisory_lock(session, f"migration-pr:{identity_key}") + + existing = await self.get(session, identity_key=identity_key) + if existing is not None: + existing.branch_name = plan.branch_name + existing.patch_fingerprint = linkage.patch_fingerprint + existing.verification_plan_fingerprint = linkage.verification_plan_fingerprint + existing.bundle_fingerprint = linkage.bundle_fingerprint + existing.base_commit_sha = linkage.base_commit_sha + existing.eligibility = plan.eligibility.value + existing.status = result.status.value + existing.pr_number = result.number + existing.pr_html_url = result.html_url + existing.reason = result.reason + existing.last_reconciled_at = now + await session.flush() + return existing, False + + row = MigrationPRModel( + identity_key=identity_key, repository=linkage.repository, change_fingerprint=linkage.change_fingerprint, + engine_version=linkage.engine_version, branch_name=plan.branch_name, + patch_fingerprint=linkage.patch_fingerprint, + verification_plan_fingerprint=linkage.verification_plan_fingerprint, + bundle_fingerprint=linkage.bundle_fingerprint, base_commit_sha=linkage.base_commit_sha, + eligibility=plan.eligibility.value, status=result.status.value, pr_number=result.number, + pr_html_url=result.html_url, reason=result.reason, created_at=now, last_reconciled_at=now, + ) + session.add(row) + await session.flush() + return row, True + + def to_domain(self, row: MigrationPRModel) -> MigrationPullRequest: + return MigrationPullRequest( + id=str(row.id), number=row.pr_number, html_url=row.pr_html_url, status=MigrationPRStatus(row.status), + linkage=MigrationPRLinkage( + change_fingerprint=row.change_fingerprint, dependency_key=None, repository=row.repository, + base_commit_sha=row.base_commit_sha, plan_fingerprint="", patch_fingerprint=row.patch_fingerprint, + verification_plan_fingerprint=row.verification_plan_fingerprint, + bundle_fingerprint=row.bundle_fingerprint, engine_version=row.engine_version, + ), + branch_name=row.branch_name, reason=row.reason, + ) + + +__all__ = ["MigrationPRStore"] diff --git a/patchfrog/migration_verification/__init__.py b/patchfrog/migration_verification/__init__.py new file mode 100644 index 0000000..98ae907 --- /dev/null +++ b/patchfrog/migration_verification/__init__.py @@ -0,0 +1,22 @@ +"""Migration Verification -- M8. + +migration plan + generated patch + exact repository state -> verification +plan -> bounded, sandboxed execution -> evidence bundle -> deterministic +outcome. Never a provider call; never treats generated code, a single +passing generic test, or AI output as proof. See +``docs/migration-verification.md``. +""" + +from patchfrog.migration_verification.domain import ( + MIGRATION_VERIFICATION_VERSION, + MigrationEvidenceBundle, + VerificationOutcome, +) +from patchfrog.migration_verification.service import run_migration_verification + +__all__ = [ + "MIGRATION_VERIFICATION_VERSION", + "MigrationEvidenceBundle", + "VerificationOutcome", + "run_migration_verification", +] diff --git a/patchfrog/migration_verification/adapters/__init__.py b/patchfrog/migration_verification/adapters/__init__.py new file mode 100644 index 0000000..56d9217 --- /dev/null +++ b/patchfrog/migration_verification/adapters/__init__.py @@ -0,0 +1,7 @@ +"""Sandboxed verification-step adapters for Migration Verification (M8.4). + +Every adapter here runs exclusively through +:class:`patchfrog.executable_verification.sandbox.VerificationSandbox` -- +mirrors :mod:`patchfrog.executable_verification.pytest_adapter`'s shape. +Never a second, parallel subprocess-execution implementation. +""" diff --git a/patchfrog/migration_verification/adapters/imports.py b/patchfrog/migration_verification/adapters/imports.py new file mode 100644 index 0000000..e4c287e --- /dev/null +++ b/patchfrog/migration_verification/adapters/imports.py @@ -0,0 +1,90 @@ +"""Import-resolution verification step (M8.4/M8.2's ``IMPORTS_RESOLVE``). + +Actually imports each modified module inside the sandbox, from the +patched workspace root -- a moved/renamed/removed import raises +``ImportError``/``ModuleNotFoundError`` immediately, exactly the failure +this requirement exists to catch. This executes the module's own +top-level code (same trust model as +:mod:`patchfrog.executable_verification.pytest_adapter`'s pytest +collection, which does the same thing for test modules) -- always inside +:class:`~patchfrog.executable_verification.sandbox.VerificationSandbox`, +never unsandboxed. +""" + +from __future__ import annotations + +import sys +from pathlib import Path, PurePosixPath + +from patchfrog.executable_verification.sandbox import ( + VerificationSandbox, + stderr_excerpt, + stdout_excerpt, +) +from patchfrog.migration_verification.domain import CheckStatus, StepEvidence, VerificationStepKind + +_PYTHON = sys.executable + +_IMPORT_SCRIPT = ( + "import importlib, sys\n" + "failures = []\n" + "for name in sys.argv[1:]:\n" + " try:\n" + " importlib.import_module(name)\n" + " except Exception as exc:\n" + " failures.append(f'{name}: {exc.__class__.__name__}: {exc}')\n" + "if failures:\n" + " print('\\n'.join(failures), file=sys.stderr)\n" + " sys.exit(1)\n" +) + + +def module_name_for_path(file_path: str) -> str | None: + """Best-effort dotted module path for a repository-relative ``.py`` + file -- ``None`` when any path component is not a valid Python + identifier (never guessed further; the caller must then leave that + file's import unverified rather than build a wrong module name).""" + + path = PurePosixPath(file_path) + if path.suffix != ".py": + return None + parts = list(path.parts[:-1]) + stem = path.stem + if stem != "__init__": + parts.append(stem) + if not parts: + return None + if any(not part.isidentifier() for part in parts): + return None + return ".".join(parts) + + +async def run_import_check( + sandbox: VerificationSandbox, *, workspace_root: Path, step_id: str, requirement_ids: tuple[str, ...], + module_names: tuple[str, ...], +) -> StepEvidence: + if not module_names: + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.IMPORT_CHECK, requirement_ids=requirement_ids, + status=CheckStatus.UNAVAILABLE, exit_code=None, + duration_ms=0.0, stdout_excerpt="", stderr_excerpt="", + detail="no resolvable module path for any modified file", + ) + result = await sandbox.run([_PYTHON, "-c", _IMPORT_SCRIPT, *module_names], cwd=workspace_root) + if result.timed_out: + status = CheckStatus.UNAVAILABLE + detail = "import check timed out" + elif result.exit_code == 0: + status = CheckStatus.PASSED + detail = f"{len(module_names)} module(s) imported cleanly" + else: + status = CheckStatus.FAILED + detail = f"import failed (exit {result.exit_code})" + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.IMPORT_CHECK, requirement_ids=requirement_ids, status=status, + exit_code=result.exit_code, duration_ms=result.duration_ms, stdout_excerpt=stdout_excerpt(result.stdout), + stderr_excerpt=stderr_excerpt(result.stderr), detail=detail, + ) + + +__all__ = ["module_name_for_path", "run_import_check"] diff --git a/patchfrog/migration_verification/adapters/mypy_adapter.py b/patchfrog/migration_verification/adapters/mypy_adapter.py new file mode 100644 index 0000000..682996b --- /dev/null +++ b/patchfrog/migration_verification/adapters/mypy_adapter.py @@ -0,0 +1,60 @@ +"""Best-effort static type check verification step (M8.4's ``TYPE_CHECK_PASSES``). + +Uses PatchFrog's own bundled ``mypy`` (already a project dependency, used +by :mod:`patchfrog.analysis.analyzers` for review) against the target +repository's modified files. Never mandatory alone for VERIFIED (see +:mod:`patchfrog.migration_verification.requirements`) -- a target +repository's own type-checking configuration/stubs are not installed in +PatchFrog's sandbox, so this is evidence, never proof by itself.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from patchfrog.executable_verification.sandbox import ( + VerificationSandbox, + stderr_excerpt, + stdout_excerpt, +) +from patchfrog.migration_verification.domain import CheckStatus, StepEvidence, VerificationStepKind + +_PYTHON = sys.executable +_EXIT_NO_ERRORS = 0 +_EXIT_TYPE_ERRORS = 1 +_EXIT_USAGE_ERROR = 2 + + +async def run_type_check( + sandbox: VerificationSandbox, *, workspace_root: Path, step_id: str, requirement_ids: tuple[str, ...], + file_paths: tuple[str, ...], +) -> StepEvidence: + if not file_paths: + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.TYPE_CHECK, requirement_ids=requirement_ids, + status=CheckStatus.NOT_RUN, exit_code=None, + duration_ms=0.0, stdout_excerpt="", stderr_excerpt="", detail="no Python files to type-check", + ) + result = await sandbox.run( + [_PYTHON, "-m", "mypy", "--ignore-missing-imports", "--no-error-summary", *file_paths], cwd=workspace_root, + ) + if result.timed_out: + status = CheckStatus.UNAVAILABLE + detail = "type check timed out" + elif result.exit_code == _EXIT_NO_ERRORS: + status = CheckStatus.PASSED + detail = f"{len(file_paths)} file(s) type-checked cleanly" + elif result.exit_code == _EXIT_TYPE_ERRORS: + status = CheckStatus.FAILED + detail = "mypy reported type errors" + else: + status = CheckStatus.UNAVAILABLE + detail = f"mypy could not run (exit {result.exit_code})" + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.TYPE_CHECK, requirement_ids=requirement_ids, status=status, + exit_code=result.exit_code, duration_ms=result.duration_ms, stdout_excerpt=stdout_excerpt(result.stdout), + stderr_excerpt=stderr_excerpt(result.stderr), detail=detail, + ) + + +__all__ = ["run_type_check"] diff --git a/patchfrog/migration_verification/adapters/syntax.py b/patchfrog/migration_verification/adapters/syntax.py new file mode 100644 index 0000000..586414d --- /dev/null +++ b/patchfrog/migration_verification/adapters/syntax.py @@ -0,0 +1,49 @@ +"""Syntax-validity verification step (M8.4/M8.2's ``SYNTAX_VALID``). + +Compiles every modified Python file with ``py_compile`` inside the +sandbox -- never a hand-rolled parser, and never a check against the +running PatchFrog process's own interpreter unsandboxed.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from patchfrog.executable_verification.sandbox import ( + VerificationSandbox, + stderr_excerpt, + stdout_excerpt, +) +from patchfrog.migration_verification.domain import CheckStatus, StepEvidence, VerificationStepKind + +_PYTHON = sys.executable + + +async def run_syntax_check( + sandbox: VerificationSandbox, *, workspace_root: Path, step_id: str, requirement_ids: tuple[str, ...], + file_paths: tuple[str, ...], +) -> StepEvidence: + if not file_paths: + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.SYNTAX_CHECK, requirement_ids=requirement_ids, + status=CheckStatus.NOT_RUN, exit_code=None, + duration_ms=0.0, stdout_excerpt="", stderr_excerpt="", detail="no Python files to check", + ) + result = await sandbox.run([_PYTHON, "-m", "py_compile", *file_paths], cwd=workspace_root) + if result.timed_out: + status = CheckStatus.UNAVAILABLE + detail = "syntax check timed out" + elif result.exit_code == 0: + status = CheckStatus.PASSED + detail = f"{len(file_paths)} file(s) compiled cleanly" + else: + status = CheckStatus.FAILED + detail = f"py_compile failed (exit {result.exit_code})" + return StepEvidence( + step_id=step_id, kind=VerificationStepKind.SYNTAX_CHECK, requirement_ids=requirement_ids, status=status, + exit_code=result.exit_code, duration_ms=result.duration_ms, stdout_excerpt=stdout_excerpt(result.stdout), + stderr_excerpt=stderr_excerpt(result.stderr), detail=detail, + ) + + +__all__ = ["run_syntax_check"] diff --git a/patchfrog/migration_verification/baseline.py b/patchfrog/migration_verification/baseline.py new file mode 100644 index 0000000..395756c --- /dev/null +++ b/patchfrog/migration_verification/baseline.py @@ -0,0 +1,104 @@ +"""Baseline-vs-patched two-state verification (M8.6). + +Proves the migration actually *fixed* something, rather than merely +passing a test that would have passed anyway. Both states are disposable +copies of the same original checkout -- the real working tree is never +mutated, and nothing is ever cloned or fetched over the network for this +comparison (the "baseline" is simply the pre-patch content of the exact +same repository state the plan was built from). + +Bounded by policy: only ever run for a small number of the +highest-confidence (``DIRECT``) targeted tests -- see +:mod:`patchfrog.migration_verification.service` for the cap -- never for +every selected test (M8.6's own "do not require baseline execution for +every case" instruction). +""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path + +from patchfrog.executable_verification.domain import VerificationOutcome as ExecOutcome +from patchfrog.executable_verification.pytest_adapter import run_pytest_verification +from patchfrog.executable_verification.sandbox import VerificationSandbox +from patchfrog.migration.domain import GeneratedPatch +from patchfrog.migration_verification.domain import ( + BaselineComparison, + BaselineComparisonOutcome, + CheckStatus, +) + +_EXEC_TO_CHECK_STATUS = { + ExecOutcome.PASSED: CheckStatus.PASSED, + ExecOutcome.CONFIRMED_FAILURE: CheckStatus.FAILED, + ExecOutcome.TIMEOUT: CheckStatus.UNAVAILABLE, + ExecOutcome.UNSUPPORTED: CheckStatus.UNAVAILABLE, + ExecOutcome.INCONCLUSIVE: CheckStatus.UNAVAILABLE, + ExecOutcome.SANDBOX_ERROR: CheckStatus.UNAVAILABLE, +} + +_OUTCOME_MATRIX = { + (CheckStatus.FAILED, CheckStatus.PASSED): BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_PASS, + (CheckStatus.PASSED, CheckStatus.PASSED): BaselineComparisonOutcome.BASELINE_PASS_PATCHED_PASS, + (CheckStatus.FAILED, CheckStatus.FAILED): BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_FAIL_DIFFERENTLY, + (CheckStatus.PASSED, CheckStatus.FAILED): BaselineComparisonOutcome.BASELINE_PASS_PATCHED_FAIL, +} + + +def _apply_patch_contents(workspace: Path, patch: GeneratedPatch) -> None: + for relative_path, content in patch.new_contents.items(): + target = workspace / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + + +async def run_baseline_comparison( + sandbox: VerificationSandbox, + *, + root: Path, + patch: GeneratedPatch, + requirement_id: str, + test_path: str, + commit_sha: str, + extra_workspace_paths: tuple[Path, ...] = (), +) -> BaselineComparison: + if not patch.is_candidate or not patch.new_contents: + return BaselineComparison( + requirement_id=requirement_id, baseline_status=CheckStatus.UNAVAILABLE, + patched_status=CheckStatus.UNAVAILABLE, outcome=BaselineComparisonOutcome.BASELINE_UNAVAILABLE, + detail="no candidate patch content to compare against a baseline", + ) + + baseline_ws = Path(tempfile.mkdtemp(prefix="patchfrog-verify-baseline-")) + patched_ws = Path(tempfile.mkdtemp(prefix="patchfrog-verify-patched-")) + try: + shutil.copytree(root, baseline_ws, dirs_exist_ok=True, symlinks=True) + shutil.copytree(root, patched_ws, dirs_exist_ok=True, symlinks=True) + for extra_path in extra_workspace_paths: + shutil.copytree(extra_path, baseline_ws, dirs_exist_ok=True, symlinks=True) + shutil.copytree(extra_path, patched_ws, dirs_exist_ok=True, symlinks=True) + _apply_patch_contents(patched_ws, patch) + + baseline_evidence = await run_pytest_verification( + sandbox, workspace_root=baseline_ws, test_target_path=test_path, commit_sha=commit_sha, + ) + patched_evidence = await run_pytest_verification( + sandbox, workspace_root=patched_ws, test_target_path=test_path, commit_sha=commit_sha, + ) + finally: + shutil.rmtree(baseline_ws, ignore_errors=True) + shutil.rmtree(patched_ws, ignore_errors=True) + + baseline_status = _EXEC_TO_CHECK_STATUS[baseline_evidence.outcome] + patched_status = _EXEC_TO_CHECK_STATUS[patched_evidence.outcome] + outcome = _OUTCOME_MATRIX.get((baseline_status, patched_status), BaselineComparisonOutcome.BASELINE_UNAVAILABLE) + detail = f"baseline={baseline_evidence.outcome.value}, patched={patched_evidence.outcome.value} on {test_path}" + return BaselineComparison( + requirement_id=requirement_id, baseline_status=baseline_status, patched_status=patched_status, + outcome=outcome, detail=detail, + ) + + +__all__ = ["run_baseline_comparison"] diff --git a/patchfrog/migration_verification/contract.py b/patchfrog/migration_verification/contract.py new file mode 100644 index 0000000..804577b --- /dev/null +++ b/patchfrog/migration_verification/contract.py @@ -0,0 +1,113 @@ +"""Deterministic contract verification (M8.7). + +Reuses M7's own already-computed +:class:`~patchfrog.migration.domain.StepResult`\\ s (the generator's own +record of whether each step's edit was applied, was already unnecessary, +or failed) rather than re-deriving the same evidence with string/regex +matching over the patched content. A step the generator itself reports +``APPLIED``/``NOT_NEEDED`` is real, parser-driven evidence that the +change was made; ``FAILED``/``SKIPPED``/``PENDING`` means it wasn't. + +The one additional deterministic check this module adds beyond the +generator's own bookkeeping: for a version-constraint requirement, the +patched manifest content is checked to actually contain the target +version string -- a safety net against a `StepResult` that claims +`APPLIED` but produced content not matching what was promised. +""" + +from __future__ import annotations + +from patchfrog.migration.domain import GeneratedPatch, MigrationPlan, MigrationStep, StepOutcome +from patchfrog.migration_verification.domain import ( + CheckStatus, + ContractCheckResult, + VerificationRequirement, + VerificationRequirementKind, +) + +_CONTRACT_REQUIREMENT_KINDS = frozenset( + { + VerificationRequirementKind.OBSOLETE_SYMBOL_REMOVED, + VerificationRequirementKind.SDK_CALL_MATCHES_CONTRACT, + VerificationRequirementKind.REQUIRED_ARGUMENT_PRESENT, + VerificationRequirementKind.VERSION_CONSTRAINT_COMPATIBLE, + } +) + +_STEP_OUTCOME_STATUS = { + StepOutcome.APPLIED: CheckStatus.PASSED, + StepOutcome.NOT_NEEDED: CheckStatus.PASSED, + StepOutcome.FAILED: CheckStatus.FAILED, + StepOutcome.SKIPPED: CheckStatus.NOT_RUN, + StepOutcome.PENDING: CheckStatus.NOT_RUN, +} + + +def verify_contract( + plan: MigrationPlan, patch: GeneratedPatch, requirements: tuple[VerificationRequirement, ...] +) -> tuple[ContractCheckResult, ...]: + steps_by_id: dict[str, MigrationStep] = {s.step_id: s for s in plan.steps} + results_by_step: dict[str, StepOutcome] = {r.step_id: r.outcome for r in patch.step_results} + + checks: list[ContractCheckResult] = [] + for requirement in requirements: + if requirement.kind not in _CONTRACT_REQUIREMENT_KINDS or requirement.source_step_id is None: + continue + step = steps_by_id.get(requirement.source_step_id) + outcome = results_by_step.get(requirement.source_step_id) + if step is None or outcome is None: + checks.append( + ContractCheckResult( + requirement_id=requirement.requirement_id, description=requirement.description, + status=CheckStatus.UNAVAILABLE, detail="no generator step result found for this requirement", + ) + ) + continue + + status = _STEP_OUTCOME_STATUS[outcome] + detail = f"generator reported step {outcome.value!r} for {step.target.location}" + + if ( + requirement.kind is VerificationRequirementKind.VERSION_CONSTRAINT_COMPATIBLE + and status is CheckStatus.PASSED + and step.version_constraint is not None + ): + content = patch.new_contents.get(step.target.file_path) + if content is None or step.version_constraint.target not in content: + status = CheckStatus.FAILED + detail = ( + f"generator reported {outcome.value!r} but {step.target.file_path} does not contain " + f"target version {step.version_constraint.target!r}" + ) + else: + detail += f"; patched manifest contains target version {step.version_constraint.target!r}" + + checks.append( + ContractCheckResult( + requirement_id=requirement.requirement_id, description=requirement.description, + status=status, detail=detail, + ) + ) + + aggregate = next( + (r for r in requirements if r.kind is VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED), None + ) + if aggregate is not None: + per_step = list(checks) + if per_step and all(c.status is CheckStatus.PASSED for c in per_step): + agg_status, agg_detail = CheckStatus.PASSED, "every contract-specific check for this migration passed" + elif any(c.status is CheckStatus.FAILED for c in per_step): + agg_status, agg_detail = CheckStatus.FAILED, "at least one contract-specific check failed" + else: + agg_status, agg_detail = CheckStatus.NOT_RUN, "one or more contract-specific checks did not run" + checks.append( + ContractCheckResult( + requirement_id=aggregate.requirement_id, description=aggregate.description, + status=agg_status, detail=agg_detail, + ) + ) + + return tuple(checks) + + +__all__ = ["verify_contract"] diff --git a/patchfrog/migration_verification/decision.py b/patchfrog/migration_verification/decision.py new file mode 100644 index 0000000..7a1e6e3 --- /dev/null +++ b/patchfrog/migration_verification/decision.py @@ -0,0 +1,85 @@ +"""Deterministic final verification decision engine (M8.11). + +No LLM decides final truth here -- every branch is a pure function of +already-computed, explainable evidence. Mirrors the evidence-combination +discipline of :class:`patchfrog.fix_verification.domain.FixEvidenceDirection`. +Precedence (highest first): REGRESSION_DETECTED > FAILED > HUMAN_REQUIRED > +VERIFIED / PARTIALLY_VERIFIED > UNVERIFIED. Contradicting evidence beats a +pending human step; a pending human step beats any "how much did we prove". +Within that: +a single strong contradicting signal wins outright; only strong, direct +evidence reaches ``VERIFIED``; weak evidence alone never promotes past +``PARTIALLY_VERIFIED``; missing evidence is ``UNVERIFIED``, never guessed. +""" + +from __future__ import annotations + +from patchfrog.migration_verification.domain import ( + EvidenceStrength, + MigrationVerificationPlan, + VerificationCoverage, + VerificationOutcome, +) +from patchfrog.migration_verification.regression import RegressionFinding + + +def decide_outcome( + plan: MigrationVerificationPlan, + *, + coverage: VerificationCoverage, + evidence_strength: EvidenceStrength, + regressions: tuple[RegressionFinding, ...], + has_unresolved_human_steps: bool, +) -> tuple[VerificationOutcome, tuple[str, ...]]: + # A regression is real, positive evidence of new damage -- it always + # wins outright, regardless of how many other requirements passed. + if regressions: + return VerificationOutcome.REGRESSION_DETECTED, tuple(f"{r.reason_code}: {r.detail}" for r in regressions) + + if not plan.requirements: + if has_unresolved_human_steps: + return VerificationOutcome.HUMAN_REQUIRED, ("this migration has no automatic steps to verify; a human decision is required",) + return VerificationOutcome.UNVERIFIED, ("nothing was generated for this migration to verify",) + + if coverage.failed_mandatory: + return VerificationOutcome.FAILED, tuple( + f"mandatory requirement failed: {requirement_id}" for requirement_id in coverage.failed_mandatory + ) + + unresolved = coverage.unresolved_mandatory + + if has_unresolved_human_steps: + # Contradicting evidence (regression/failed) has already won above. + # Past that point, a step the engine could not automate means the + # migration is not done until a human completes it -- that outranks + # PARTIALLY_VERIFIED/UNVERIFIED/VERIFIED, which only describe how + # much of the *automatable* part could be proven. + if unresolved: + reasons = tuple(f"unresolved mandatory requirement: {requirement_id}" for requirement_id in unresolved) + return VerificationOutcome.HUMAN_REQUIRED, ( + "the plan has unresolved human-required steps", *reasons, + ) + return VerificationOutcome.HUMAN_REQUIRED, ( + "all automatable requirements were verified, but the plan has unresolved human-required steps", + ) + + if not unresolved: + if evidence_strength in (EvidenceStrength.STRONG, EvidenceStrength.MODERATE): + return VerificationOutcome.VERIFIED, ( + "all mandatory requirements satisfied", f"evidence strength: {evidence_strength.value}", + ) + return VerificationOutcome.PARTIALLY_VERIFIED, ( + "mandatory requirements formally satisfied, but only weak or no supporting evidence was gathered", + ) + + if coverage.satisfied_requirement_ids: + return VerificationOutcome.PARTIALLY_VERIFIED, tuple( + f"unresolved mandatory requirement: {requirement_id}" for requirement_id in unresolved + ) + + return VerificationOutcome.UNVERIFIED, tuple( + f"unresolved mandatory requirement: {requirement_id}" for requirement_id in unresolved + ) or ("insufficient evidence to verify this migration",) + + +__all__ = ["decide_outcome"] diff --git a/patchfrog/migration_verification/domain.py b/patchfrog/migration_verification/domain.py new file mode 100644 index 0000000..f735fec --- /dev/null +++ b/patchfrog/migration_verification/domain.py @@ -0,0 +1,320 @@ +"""Pure domain model for Migration Verification -- M8. + +Answers, for one generated migration patch: "what evidence is needed to +show that this migration actually restores compatibility, and what did +PatchFrog actually gather?" No I/O, no LLM -- mirrors every other +Intelligence package's own ``domain.py`` role. + +**Governing rule** (mirrors the evidence-combination discipline +:class:`patchfrog.fix_verification.domain.FixEvidenceDirection` reached +only after two real correction rounds -- see +``validation/agent_handoff/latest-summary.md``): a single strong +contradicting signal (a regression, a failed mandatory contract check) +always wins outright. Only strong, direct evidence may reach +``VERIFIED``. Any combination of only weak/generic evidence never +promotes above ``PARTIALLY_VERIFIED``. Missing required evidence is +``UNVERIFIED``, never guessed in either direction. See +:mod:`patchfrog.migration_verification.decision` for the actual rule. + +A verification result must explicitly distinguish evidence collected, +checks passed, checks failed, checks not run, checks unavailable, and +residual uncertainty -- see :class:`CheckStatus` and +:class:`VerificationCoverage`. +""" + +from __future__ import annotations + +import hashlib +import json +from dataclasses import dataclass, field +from enum import StrEnum + +from patchfrog.migration.domain import ResidualRisk + +#: Bumped whenever requirement-generation, test-selection, execution-plan, +#: baseline-comparison, contract-verification, regression-detection or +#: decision-engine logic changes materially enough that a prior evidence +#: bundle can no longer be considered equivalent to what re-running now +#: would produce. Independent of MIGRATION_ENGINE_VERSION (the plan/patch +#: this verifies) and every review-side version constant. +MIGRATION_VERIFICATION_VERSION = 1 + +#: Bounded plan -- never "run everything" by default (M8.2). +MAX_VERIFICATION_STEPS_PER_PLAN = 12 +MAX_TARGETED_TESTS_PER_PLAN = 8 + +#: A single step's own wall-clock ceiling and the whole plan's total +#: ceiling -- mirrors patchfrog.executable_verification.domain's own +#: per-attempt / per-run budget pattern. +MAX_STEP_SECONDS = 30.0 +MAX_TOTAL_VERIFICATION_SECONDS = 180.0 + +#: Bounds the evidence excerpt kept from stdout/stderr per step -- a +#: short excerpt for the decision engine/evidence bundle, never a log dump. +MAX_STEP_STDOUT_EXCERPT_BYTES = 4096 +MAX_STEP_STDERR_EXCERPT_BYTES = 4096 + + +class VerificationRequirementKind(StrEnum): + SYNTAX_VALID = "syntax_valid" + IMPORTS_RESOLVE = "imports_resolve" + SDK_CALL_MATCHES_CONTRACT = "sdk_call_matches_contract" + OBSOLETE_SYMBOL_REMOVED = "obsolete_symbol_removed" + REQUIRED_ARGUMENT_PRESENT = "required_argument_present" + VERSION_CONSTRAINT_COMPATIBLE = "version_constraint_compatible" + TYPE_CHECK_PASSES = "type_check_passes" + UNIT_TESTS_PASS = "unit_tests_pass" + INTEGRATION_TESTS_PASS = "integration_tests_pass" + CONTRACT_COMPATIBILITY_RESTORED = "contract_compatibility_restored" + CONSUMER_BEHAVIOR_PRESERVED = "consumer_behavior_preserved" + + +class VerificationStepKind(StrEnum): + SYNTAX_CHECK = "syntax_check" + IMPORT_CHECK = "import_check" + TYPE_CHECK = "type_check" + STATIC_ANALYSIS = "static_analysis" + TARGETED_UNIT_TEST = "targeted_unit_test" + TARGETED_INTEGRATION_TEST = "targeted_integration_test" + CONTRACT_ASSERTION = "contract_assertion" + PACKAGE_RESOLUTION_CHECK = "package_resolution_check" + GENERATED_CLIENT_CHECK = "generated_client_check" + CUSTOM_COMMAND = "custom_command" + + +class TestSelectionClass(StrEnum): + """Never claimed to be a perfect mapping -- see + :mod:`patchfrog.migration_verification.test_selection`'s module + docstring for exactly what evidence backs each value.""" + + DIRECT = "direct" + TRANSITIVE = "transitive" + FALLBACK = "fallback" + UNKNOWN = "unknown" + + +class CheckStatus(StrEnum): + """A requirement/step's own bounded outcome -- distinguishes evidence + that was actually gathered from evidence that simply never existed to + gather (M8.1's explicit requirement).""" + + PASSED = "passed" + FAILED = "failed" + NOT_RUN = "not_run" + UNAVAILABLE = "unavailable" + + +class BaselineComparisonOutcome(StrEnum): + BASELINE_FAIL_PATCHED_PASS = "baseline_fail_patched_pass" + BASELINE_PASS_PATCHED_PASS = "baseline_pass_patched_pass" + BASELINE_FAIL_PATCHED_FAIL_DIFFERENTLY = "baseline_fail_patched_fail_differently" + BASELINE_PASS_PATCHED_FAIL = "baseline_pass_patched_fail" + BASELINE_UNAVAILABLE = "baseline_unavailable" + + +class EvidenceStrength(StrEnum): + """Never let many weak checks masquerade as one strong proof (M8.10) + -- the decision engine only ever promotes to ``STRONG`` from a real + direct signal, not an accumulation of ``WEAK`` ones.""" + + STRONG = "strong" + MODERATE = "moderate" + WEAK = "weak" + NONE = "none" + + +class VerificationOutcome(StrEnum): + VERIFIED = "verified" + PARTIALLY_VERIFIED = "partially_verified" + UNVERIFIED = "unverified" + FAILED = "failed" + REGRESSION_DETECTED = "regression_detected" + HUMAN_REQUIRED = "human_required" + + +@dataclass(frozen=True, slots=True) +class VerificationRequirement: + requirement_id: str + kind: VerificationRequirementKind + description: str + #: A failed/unresolved mandatory requirement blocks VERIFIED; a + #: non-mandatory one only ever affects evidence strength/residual risk. + mandatory: bool + diff_item_keys: tuple[str, ...] = () + usage_site_key: str | None = None + #: The exact :class:`patchfrog.migration.domain.MigrationStep` this + #: requirement traces back to, when it corresponds to one step + #: (``None`` for universal/aggregate requirements) -- lets + #: :mod:`patchfrog.migration_verification.contract` reuse the + #: generator's own :class:`~patchfrog.migration.domain.StepResult` + #: directly instead of re-deriving the same evidence with string + #: matching. + source_step_id: str | None = None + + +@dataclass(frozen=True, slots=True) +class VerificationStep: + step_id: str + kind: VerificationStepKind + requirement_ids: tuple[str, ...] + #: An explicit argv -- never a shell string, never caller-assembled + #: interpolation. Executed only through + #: :class:`patchfrog.executable_verification.sandbox.VerificationSandbox`. + command: tuple[str, ...] + timeout_seconds: float + reason: str + blocks_verified: bool + #: Set only for ``TARGETED_UNIT_TEST``/``TARGETED_INTEGRATION_TEST`` + #: steps -- carries the selection's own DIRECT/TRANSITIVE/FALLBACK + #: classification through to evidence-strength scoring (M8.10) + #: without re-parsing ``reason``. + test_classification: TestSelectionClass | None = None + + +@dataclass(frozen=True, slots=True) +class TestSelection: + test_path: str + classification: TestSelectionClass + reason: str + + +@dataclass(frozen=True, slots=True) +class MigrationVerificationPlan: + change_fingerprint: str + patch_fingerprint: str + requirements: tuple[VerificationRequirement, ...] + steps: tuple[VerificationStep, ...] + test_selections: tuple[TestSelection, ...] + engine_version: int = MIGRATION_VERIFICATION_VERSION + + @property + def mandatory_requirement_ids(self) -> frozenset[str]: + return frozenset(r.requirement_id for r in self.requirements if r.mandatory) + + def fingerprint(self) -> str: + payload = { + "engine": self.engine_version, + "change": self.change_fingerprint, + "patch": self.patch_fingerprint, + "requirements": sorted(r.requirement_id for r in self.requirements), + "steps": sorted(s.step_id for s in self.steps), + } + return hashlib.sha256(json.dumps(payload, sort_keys=True).encode()).hexdigest() + + +@dataclass(frozen=True, slots=True) +class StepEvidence: + step_id: str + kind: VerificationStepKind + requirement_ids: tuple[str, ...] + status: CheckStatus + exit_code: int | None + duration_ms: float + stdout_excerpt: str + stderr_excerpt: str + detail: str + + +@dataclass(frozen=True, slots=True) +class ContractCheckResult: + requirement_id: str + description: str + status: CheckStatus + detail: str + + +@dataclass(frozen=True, slots=True) +class BaselineComparison: + requirement_id: str + baseline_status: CheckStatus + patched_status: CheckStatus + outcome: BaselineComparisonOutcome + detail: str + + +@dataclass(frozen=True, slots=True) +class VerificationCoverage: + mandatory_requirement_ids: tuple[str, ...] + satisfied_requirement_ids: tuple[str, ...] + failed_requirement_ids: tuple[str, ...] + not_run_requirement_ids: tuple[str, ...] + unavailable_requirement_ids: tuple[str, ...] + + @property + def unresolved_mandatory(self) -> tuple[str, ...]: + satisfied = set(self.satisfied_requirement_ids) + return tuple(r for r in self.mandatory_requirement_ids if r not in satisfied) + + @property + def failed_mandatory(self) -> tuple[str, ...]: + failed = set(self.failed_requirement_ids) + return tuple(r for r in self.mandatory_requirement_ids if r in failed) + + +@dataclass(frozen=True, slots=True) +class MigrationEvidenceBundle: + """First-class, reproducible, machine-readable evidence bundle + (M8.9). Persists bounded summaries, hashes, exit status and timing -- + never raw logs beyond a short bounded excerpt, never secrets.""" + + version: int + change_fingerprint: str + repository: str + repository_head_sha: str + patch_fingerprint: str + plan_fingerprint: str + plan: MigrationVerificationPlan + step_evidence: tuple[StepEvidence, ...] + contract_checks: tuple[ContractCheckResult, ...] + baseline_comparisons: tuple[BaselineComparison, ...] + coverage: VerificationCoverage + evidence_strength: EvidenceStrength + residual_risk: ResidualRisk + outcome: VerificationOutcome + outcome_reasons: tuple[str, ...] + generated_at_iso: str + unresolved_requirement_ids: tuple[str, ...] = field(default_factory=tuple) + + @property + def bundle_fingerprint(self) -> str: + """Ties this exact evidence to an exact patch/change/repo state -- + never reused for a different patch (M9.8's integrity check).""" + + payload = { + "version": self.version, + "change": self.change_fingerprint, + "repository": self.repository, + "head_sha": self.repository_head_sha, + "patch": self.patch_fingerprint, + "plan": self.plan_fingerprint, + "outcome": self.outcome.value, + } + return hashlib.sha256(json.dumps(payload, sort_keys=True).encode()).hexdigest() + + +__all__ = [ + "MAX_STEP_SECONDS", + "MAX_STEP_STDERR_EXCERPT_BYTES", + "MAX_STEP_STDOUT_EXCERPT_BYTES", + "MAX_TARGETED_TESTS_PER_PLAN", + "MAX_TOTAL_VERIFICATION_SECONDS", + "MAX_VERIFICATION_STEPS_PER_PLAN", + "MIGRATION_VERIFICATION_VERSION", + "BaselineComparison", + "BaselineComparisonOutcome", + "CheckStatus", + "ContractCheckResult", + "EvidenceStrength", + "MigrationEvidenceBundle", + "MigrationVerificationPlan", + "ResidualRisk", + "StepEvidence", + "TestSelection", + "TestSelectionClass", + "VerificationCoverage", + "VerificationOutcome", + "VerificationRequirement", + "VerificationRequirementKind", + "VerificationStep", + "VerificationStepKind", +] diff --git a/patchfrog/migration_verification/evidence.py b/patchfrog/migration_verification/evidence.py new file mode 100644 index 0000000..509d93d --- /dev/null +++ b/patchfrog/migration_verification/evidence.py @@ -0,0 +1,173 @@ +"""Evidence strength assignment (M8.10) and evidence-bundle assembly (M8.9). + +Never let many weak checks masquerade as one strong proof: strength is +assigned from the single best piece of evidence actually gathered, not +from how many weak signals accumulated. +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from patchfrog.migration_verification.domain import ( + MIGRATION_VERIFICATION_VERSION, + BaselineComparison, + BaselineComparisonOutcome, + CheckStatus, + ContractCheckResult, + EvidenceStrength, + MigrationEvidenceBundle, + MigrationVerificationPlan, + ResidualRisk, + StepEvidence, + VerificationCoverage, + VerificationOutcome, + VerificationStepKind, +) + + +def compute_evidence_strength( + *, + step_evidence: tuple[StepEvidence, ...], + contract_checks: tuple[ContractCheckResult, ...], + baseline_comparisons: tuple[BaselineComparison, ...], +) -> EvidenceStrength: + if any(c.outcome is BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_PASS for c in baseline_comparisons): + return EvidenceStrength.STRONG + mandatory_contract_checks = list(contract_checks) + if mandatory_contract_checks and all(c.status is CheckStatus.PASSED for c in mandatory_contract_checks): + return EvidenceStrength.STRONG + + if any(c.outcome is BaselineComparisonOutcome.BASELINE_PASS_PATCHED_PASS for c in baseline_comparisons): + return EvidenceStrength.MODERATE + direct_or_transitive_tests_passed = any( + e.kind is VerificationStepKind.TARGETED_UNIT_TEST and e.status is CheckStatus.PASSED + for e in step_evidence + ) + if direct_or_transitive_tests_passed: + return EvidenceStrength.MODERATE + + if any(e.kind is VerificationStepKind.TYPE_CHECK and e.status is CheckStatus.PASSED for e in step_evidence): + return EvidenceStrength.WEAK + if any(e.status is CheckStatus.PASSED for e in step_evidence): + return EvidenceStrength.WEAK + return EvidenceStrength.NONE + + +def build_coverage( + plan: MigrationVerificationPlan, + *, + step_evidence: tuple[StepEvidence, ...], + contract_checks: tuple[ContractCheckResult, ...], +) -> VerificationCoverage: + blocks_verified_by_step = {s.step_id: s.blocks_verified for s in plan.steps} + status_by_requirement: dict[str, CheckStatus] = {} + for evidence in step_evidence: + status = evidence.status + if status is CheckStatus.FAILED and not blocks_verified_by_step.get(evidence.step_id, True): + # A non-blocking step (e.g. a FALLBACK-classified targeted + # test -- M8.3's own weak-evidence tier, or the best-effort + # type check) failing is never treated as proof a requirement + # is unmet: that would let an unrelated, pre-existing failure + # ("unrelated-test trap", M8.14) force a hard + # FAILED/REGRESSION_DETECTED verdict. The raw evidence (with + # its own detail/excerpt) is still fully preserved in + # ``step_evidence`` -- only the aggregated per-requirement + # coverage treats it as inconclusive rather than damning. + status = CheckStatus.NOT_RUN + for requirement_id in evidence.requirement_ids: + status_by_requirement[requirement_id] = _combine(status_by_requirement.get(requirement_id), status) + for check in contract_checks: + status_by_requirement[check.requirement_id] = _combine(status_by_requirement.get(check.requirement_id), check.status) + + satisfied: list[str] = [] + failed: list[str] = [] + not_run: list[str] = [] + unavailable: list[str] = [] + for requirement in plan.requirements: + status = status_by_requirement.get(requirement.requirement_id, CheckStatus.NOT_RUN) + if status is CheckStatus.PASSED: + satisfied.append(requirement.requirement_id) + elif status is CheckStatus.FAILED: + failed.append(requirement.requirement_id) + elif status is CheckStatus.UNAVAILABLE: + unavailable.append(requirement.requirement_id) + else: + not_run.append(requirement.requirement_id) + + return VerificationCoverage( + mandatory_requirement_ids=tuple(sorted(plan.mandatory_requirement_ids)), + satisfied_requirement_ids=tuple(sorted(satisfied)), + failed_requirement_ids=tuple(sorted(failed)), + not_run_requirement_ids=tuple(sorted(not_run)), + unavailable_requirement_ids=tuple(sorted(unavailable)), + ) + + +def _combine(existing: CheckStatus | None, new: CheckStatus) -> CheckStatus: + """A requirement backed by more than one piece of evidence (rare, + e.g. a requirement both a step and a contract check reference) never + silently drops a failure in favor of a later pass.""" + + if existing is None: + return new + if CheckStatus.FAILED in (existing, new): + return CheckStatus.FAILED + if CheckStatus.PASSED in (existing, new): + return CheckStatus.PASSED + if CheckStatus.UNAVAILABLE in (existing, new): + return CheckStatus.UNAVAILABLE + return existing + + +def compute_residual_risk( + coverage: VerificationCoverage, *, evidence_strength: EvidenceStrength, has_unresolved_human_steps: bool +) -> ResidualRisk: + if coverage.failed_mandatory or has_unresolved_human_steps: + return ResidualRisk.HIGH + if coverage.unresolved_mandatory or evidence_strength in (EvidenceStrength.WEAK, EvidenceStrength.NONE): + return ResidualRisk.MEDIUM + return ResidualRisk.LOW + + +def build_evidence_bundle( + *, + plan: MigrationVerificationPlan, + repository: str, + repository_head_sha: str, + step_evidence: tuple[StepEvidence, ...], + contract_checks: tuple[ContractCheckResult, ...], + baseline_comparisons: tuple[BaselineComparison, ...], + coverage: VerificationCoverage, + evidence_strength: EvidenceStrength, + residual_risk: ResidualRisk, + outcome: VerificationOutcome, + outcome_reasons: tuple[str, ...], +) -> MigrationEvidenceBundle: + return MigrationEvidenceBundle( + version=MIGRATION_VERIFICATION_VERSION, + change_fingerprint=plan.change_fingerprint, + repository=repository, + repository_head_sha=repository_head_sha, + patch_fingerprint=plan.patch_fingerprint, + plan_fingerprint=plan.fingerprint(), + plan=plan, + step_evidence=step_evidence, + contract_checks=contract_checks, + baseline_comparisons=baseline_comparisons, + coverage=coverage, + evidence_strength=evidence_strength, + residual_risk=residual_risk, + outcome=outcome, + outcome_reasons=outcome_reasons, + generated_at_iso=datetime.now(UTC).isoformat(), + unresolved_requirement_ids=coverage.unresolved_mandatory, + ) + + +__all__ = [ + "build_coverage", + "build_evidence_bundle", + "compute_evidence_strength", + "compute_residual_risk", +] diff --git a/patchfrog/migration_verification/execution_plan.py b/patchfrog/migration_verification/execution_plan.py new file mode 100644 index 0000000..85c687d --- /dev/null +++ b/patchfrog/migration_verification/execution_plan.py @@ -0,0 +1,124 @@ +"""Bounded, safe execution plan (M8.4). + +Builds the fixed sequence of :class:`~patchfrog.migration_verification.domain.VerificationStep`\\ s +for a migration's requirements -- every step's ``command`` is an explicit, +pre-built argv (never a shell string, never caller-assembled at execution +time), and the whole plan is capped at +:data:`~patchfrog.migration_verification.domain.MAX_VERIFICATION_STEPS_PER_PLAN`. +Contract-specific requirements (M8.7) are deliberately **not** steps here +-- they are verified directly from the patch's own already-computed +:class:`~patchfrog.migration.domain.StepResult`\\ s in +:mod:`patchfrog.migration_verification.contract`, never a second, +string-matching re-derivation. +""" + +from __future__ import annotations + +import hashlib +import sys + +from patchfrog.migration.domain import GeneratedPatch +from patchfrog.migration_verification.adapters.imports import module_name_for_path +from patchfrog.migration_verification.domain import ( + MAX_STEP_SECONDS, + MAX_VERIFICATION_STEPS_PER_PLAN, + TestSelection, + TestSelectionClass, + VerificationRequirement, + VerificationRequirementKind, + VerificationStep, + VerificationStepKind, +) + +_PYTHON = sys.executable + + +def _step_id(*parts: str) -> str: + return hashlib.sha256("|".join(parts).encode()).hexdigest()[:16] + + +def _python_files(patch: GeneratedPatch) -> tuple[str, ...]: + return tuple(sorted(f for f in patch.modified_files if f.endswith(".py"))) + + +def build_execution_plan( + requirements: tuple[VerificationRequirement, ...], + *, + patch: GeneratedPatch, + test_selections: tuple[TestSelection, ...], +) -> tuple[VerificationStep, ...]: + by_kind = {r.kind: r for r in requirements} + steps: list[VerificationStep] = [] + python_files = _python_files(patch) + + syntax_req = by_kind.get(VerificationRequirementKind.SYNTAX_VALID) + if syntax_req is not None and python_files: + steps.append( + VerificationStep( + step_id=_step_id("syntax", syntax_req.requirement_id), + kind=VerificationStepKind.SYNTAX_CHECK, + requirement_ids=(syntax_req.requirement_id,), + command=(_PYTHON, "-m", "py_compile", *python_files), + timeout_seconds=MAX_STEP_SECONDS, + reason="every file the patch modified must remain syntactically valid", + blocks_verified=True, + ) + ) + + imports_req = by_kind.get(VerificationRequirementKind.IMPORTS_RESOLVE) + if imports_req is not None and python_files: + module_names = tuple(sorted({m for f in python_files if (m := module_name_for_path(f)) is not None})) + steps.append( + VerificationStep( + step_id=_step_id("imports", imports_req.requirement_id), + kind=VerificationStepKind.IMPORT_CHECK, + requirement_ids=(imports_req.requirement_id,), + command=(_PYTHON, "-c", "", *module_names), + timeout_seconds=MAX_STEP_SECONDS, + reason="a moved/renamed/removed import must fail import, not merely fail to parse", + blocks_verified=False, + ) + ) + + type_req = by_kind.get(VerificationRequirementKind.TYPE_CHECK_PASSES) + if type_req is not None and python_files: + steps.append( + VerificationStep( + step_id=_step_id("typecheck", type_req.requirement_id), + kind=VerificationStepKind.TYPE_CHECK, + requirement_ids=(type_req.requirement_id,), + command=(_PYTHON, "-m", "mypy", "--ignore-missing-imports", *python_files), + timeout_seconds=MAX_STEP_SECONDS, + reason="best-effort static type check of the modified files", + blocks_verified=False, + ) + ) + + unit_tests_req = by_kind.get(VerificationRequirementKind.UNIT_TESTS_PASS) + if unit_tests_req is not None: + runnable = [s for s in test_selections if s.classification is not TestSelectionClass.UNKNOWN] + for selection in runnable: + if len(steps) >= MAX_VERIFICATION_STEPS_PER_PLAN: + break + steps.append( + VerificationStep( + step_id=_step_id("test", unit_tests_req.requirement_id, selection.test_path), + kind=VerificationStepKind.TARGETED_UNIT_TEST, + requirement_ids=(unit_tests_req.requirement_id,), + command=(_PYTHON, "-m", "pytest", "-q", selection.test_path), + timeout_seconds=MAX_STEP_SECONDS, + reason=f"{selection.classification.value}: {selection.reason}", + #: Only a DIRECT/TRANSITIVE selection blocks VERIFIED + #: on failure -- a FALLBACK selection is weak evidence + #: by construction (M8.3), never strong enough alone + #: to withhold VERIFIED on a failure that may be + #: unrelated to this migration. + blocks_verified=selection.classification in (TestSelectionClass.DIRECT, TestSelectionClass.TRANSITIVE), + test_classification=selection.classification, + ) + ) + + return tuple(steps[:MAX_VERIFICATION_STEPS_PER_PLAN]) + + +__all__ = ["build_execution_plan"] diff --git a/patchfrog/migration_verification/regression.py b/patchfrog/migration_verification/regression.py new file mode 100644 index 0000000..a9ae3b8 --- /dev/null +++ b/patchfrog/migration_verification/regression.py @@ -0,0 +1,78 @@ +"""Regression detection (M8.8) -- new damage the migration itself introduced. + +Deliberately reuses evidence already gathered by the rest of the M8 +pipeline (step evidence, contract checks, baseline comparisons, the +patch's own safety-gate results) rather than executing anything new. +``REGRESSION_DETECTED`` is always distinct from ``UNVERIFIED`` (M8.8's +own explicit requirement) -- a regression finding here means real, +positive evidence of new damage, never merely "we couldn't prove it's +fine". +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from patchfrog.migration.domain import GeneratedPatch, MigrationPlan +from patchfrog.migration_verification.domain import ( + BaselineComparison, + BaselineComparisonOutcome, + CheckStatus, + ContractCheckResult, + StepEvidence, + VerificationStepKind, +) + + +@dataclass(frozen=True, slots=True) +class RegressionFinding: + reason_code: str + detail: str + + +def detect_regressions( + plan: MigrationPlan, + patch: GeneratedPatch, + *, + step_evidence: tuple[StepEvidence, ...], + contract_checks: tuple[ContractCheckResult, ...], + baseline_comparisons: tuple[BaselineComparison, ...], +) -> tuple[RegressionFinding, ...]: + findings: list[RegressionFinding] = [] + + for gate in patch.safety: + if not gate.passed: + findings.append(RegressionFinding("safety_gate_regression", f"{gate.gate}: {gate.detail}")) + + for evidence in step_evidence: + if evidence.status is CheckStatus.FAILED and evidence.kind is VerificationStepKind.SYNTAX_CHECK: + findings.append(RegressionFinding("syntax_failure", evidence.detail)) + # Deliberately NOT a regression signal: TYPE_CHECK failure alone. + # PatchFrog's sandbox runs its own bundled mypy against the target + # repository without that repository's own type-checking + # configuration/stubs (see requirements.py's TYPE_CHECK_PASSES + # docstring) -- a failure here does not independently prove the + # migration introduced anything (CLAUDE.md's false-positive + # discipline: a signal that cannot independently prove a defect + # must never become one on its own). It remains WEAK evidence via + # :mod:`patchfrog.migration_verification.evidence`, which caps + # the achievable outcome at PARTIALLY_VERIFIED rather than + # promoting it to a publication-blocking REGRESSION_DETECTED. + + for comparison in baseline_comparisons: + if comparison.outcome is BaselineComparisonOutcome.BASELINE_PASS_PATCHED_FAIL: + findings.append(RegressionFinding("targeted_test_regression", comparison.detail)) + + for check in contract_checks: + if check.status is CheckStatus.FAILED: + findings.append(RegressionFinding("contract_mismatch", f"{check.description}: {check.detail}")) + + target_files = {s.target.file_path for s in plan.steps} + unrelated = sorted(set(patch.modified_files) - target_files) + for path in unrelated: + findings.append(RegressionFinding("unrelated_file_modified", f"{path} was modified outside the migration's own step targets")) + + return tuple(findings) + + +__all__ = ["RegressionFinding", "detect_regressions"] diff --git a/patchfrog/migration_verification/report.py b/patchfrog/migration_verification/report.py new file mode 100644 index 0000000..6c1dd75 --- /dev/null +++ b/patchfrog/migration_verification/report.py @@ -0,0 +1,143 @@ +"""Machine-readable (JSON) and human-readable views of M8 evidence bundles. + +The same dictionaries back ``--json`` CLI output and the bounded JSON +columns in :mod:`patchfrog.migration_verification.store`, mirroring +:mod:`patchfrog.migration.report`'s role for M7. +""" + +from __future__ import annotations + +from typing import Any + +from patchfrog.migration_verification.domain import ( + BaselineComparison, + ContractCheckResult, + MigrationEvidenceBundle, + MigrationVerificationPlan, + StepEvidence, + TestSelection, + VerificationCoverage, + VerificationRequirement, + VerificationStep, +) + + +def requirement_to_dict(requirement: VerificationRequirement) -> dict[str, Any]: + return { + "requirement_id": requirement.requirement_id, "kind": requirement.kind.value, + "description": requirement.description, "mandatory": requirement.mandatory, + "diff_item_keys": list(requirement.diff_item_keys), "usage_site_key": requirement.usage_site_key, + "source_step_id": requirement.source_step_id, + } + + +def step_to_dict(step: VerificationStep) -> dict[str, Any]: + return { + "step_id": step.step_id, "kind": step.kind.value, "requirement_ids": list(step.requirement_ids), + "command": list(step.command), "timeout_seconds": step.timeout_seconds, "reason": step.reason, + "blocks_verified": step.blocks_verified, + "test_classification": step.test_classification.value if step.test_classification else None, + } + + +def test_selection_to_dict(selection: TestSelection) -> dict[str, Any]: + return {"test_path": selection.test_path, "classification": selection.classification.value, "reason": selection.reason} + + +def plan_to_dict(plan: MigrationVerificationPlan) -> dict[str, Any]: + return { + "change_fingerprint": plan.change_fingerprint, "patch_fingerprint": plan.patch_fingerprint, + "engine_version": plan.engine_version, "fingerprint": plan.fingerprint(), + "requirements": [requirement_to_dict(r) for r in plan.requirements], + "steps": [step_to_dict(s) for s in plan.steps], + "test_selections": [test_selection_to_dict(s) for s in plan.test_selections], + } + + +def step_evidence_to_dict(evidence: StepEvidence) -> dict[str, Any]: + return { + "step_id": evidence.step_id, "kind": evidence.kind.value, "requirement_ids": list(evidence.requirement_ids), + "status": evidence.status.value, "exit_code": evidence.exit_code, "duration_ms": evidence.duration_ms, + "stdout_excerpt": evidence.stdout_excerpt, "stderr_excerpt": evidence.stderr_excerpt, "detail": evidence.detail, + } + + +def contract_check_to_dict(check: ContractCheckResult) -> dict[str, Any]: + return { + "requirement_id": check.requirement_id, "description": check.description, "status": check.status.value, + "detail": check.detail, + } + + +def baseline_comparison_to_dict(comparison: BaselineComparison) -> dict[str, Any]: + return { + "requirement_id": comparison.requirement_id, "baseline_status": comparison.baseline_status.value, + "patched_status": comparison.patched_status.value, "outcome": comparison.outcome.value, + "detail": comparison.detail, + } + + +def coverage_to_dict(coverage: VerificationCoverage) -> dict[str, Any]: + return { + "mandatory_requirement_ids": list(coverage.mandatory_requirement_ids), + "satisfied_requirement_ids": list(coverage.satisfied_requirement_ids), + "failed_requirement_ids": list(coverage.failed_requirement_ids), + "not_run_requirement_ids": list(coverage.not_run_requirement_ids), + "unavailable_requirement_ids": list(coverage.unavailable_requirement_ids), + "unresolved_mandatory": list(coverage.unresolved_mandatory), + } + + +def bundle_to_dict(bundle: MigrationEvidenceBundle) -> dict[str, Any]: + return { + "version": bundle.version, "bundle_fingerprint": bundle.bundle_fingerprint, + "change_fingerprint": bundle.change_fingerprint, "repository": bundle.repository, + "repository_head_sha": bundle.repository_head_sha, "patch_fingerprint": bundle.patch_fingerprint, + "plan_fingerprint": bundle.plan_fingerprint, "plan": plan_to_dict(bundle.plan), + "step_evidence": [step_evidence_to_dict(e) for e in bundle.step_evidence], + "contract_checks": [contract_check_to_dict(c) for c in bundle.contract_checks], + "baseline_comparisons": [baseline_comparison_to_dict(b) for b in bundle.baseline_comparisons], + "coverage": coverage_to_dict(bundle.coverage), + "evidence_strength": bundle.evidence_strength.value, "residual_risk": bundle.residual_risk.value, + "outcome": bundle.outcome.value, "outcome_reasons": list(bundle.outcome_reasons), + "unresolved_requirement_ids": list(bundle.unresolved_requirement_ids), + "generated_at": bundle.generated_at_iso, + } + + +def render_bundle_text(bundle: MigrationEvidenceBundle) -> str: + lines = [f"Verification outcome: {bundle.outcome.value.upper()}"] + for reason in bundle.outcome_reasons: + lines.append(f" - {reason}") + if bundle.contract_checks: + lines.append("Contract:") + for check in bundle.contract_checks: + mark = "PASS" if check.status.value == "passed" else check.status.value.upper() + lines.append(f" {mark} -- {check.description}") + test_steps = [e for e in bundle.step_evidence if e.kind.value in ("targeted_unit_test", "targeted_integration_test")] + if test_steps: + passed = sum(1 for e in test_steps if e.status.value == "passed") + lines.append(f"Targeted tests: {passed}/{len(test_steps)} passed") + type_checks = [e for e in bundle.step_evidence if e.kind.value == "type_check"] + if type_checks: + lines.append(f"Type checks: {type_checks[0].status.value.upper()}") + if bundle.baseline_comparisons: + lines.append("Baseline comparison:") + for comparison in bundle.baseline_comparisons: + lines.append(f" {comparison.baseline_status.value} -> {comparison.patched_status.value} ({comparison.outcome.value})") + lines.append(f"Residual risk: {bundle.residual_risk.value.upper()}") + return "\n".join(lines) + "\n" + + +__all__ = [ + "baseline_comparison_to_dict", + "bundle_to_dict", + "contract_check_to_dict", + "coverage_to_dict", + "plan_to_dict", + "render_bundle_text", + "requirement_to_dict", + "step_evidence_to_dict", + "step_to_dict", + "test_selection_to_dict", +] diff --git a/patchfrog/migration_verification/requirements.py b/patchfrog/migration_verification/requirements.py new file mode 100644 index 0000000..46d3ad0 --- /dev/null +++ b/patchfrog/migration_verification/requirements.py @@ -0,0 +1,190 @@ +"""Verification requirement generation from real migration context (M8.2). + +Every requirement traces back to either a universal "the patch must not be +obviously broken" concern (syntax/imports) or a specific +:class:`~patchfrog.migration.domain.MigrationStep` this exact patch +applied -- never invented, never "run everything" by default. +""" + +from __future__ import annotations + +import hashlib + +from patchfrog.migration.domain import MigrationPlan, MigrationStep, MigrationStrategy +from patchfrog.migration_verification.domain import ( + TestSelection, + VerificationRequirement, + VerificationRequirementKind, +) + +#: Strategies whose fix is "the SDK call now matches the new contract" -- +#: verified by confirming the old, obsolete call shape no longer appears +#: and (where the contract gives one) the replacement does. +_CONTRACT_CALL_STRATEGIES = frozenset( + { + MigrationStrategy.RENAME_SYMBOL, + MigrationStrategy.REPLACE_ENDPOINT, + MigrationStrategy.REPLACE_REMOVED_API, + MigrationStrategy.RENAME_PARAMETER, + MigrationStrategy.REPLACE_ENUM_VALUE, + MigrationStrategy.ADAPT_RETURN_FIELD, + MigrationStrategy.ADAPT_RESPONSE, + MigrationStrategy.UPDATE_AUTH, + MigrationStrategy.REMOVE_ARGUMENT, + } +) +_OBSOLETE_SYMBOL_STRATEGIES = frozenset( + {MigrationStrategy.RENAME_SYMBOL, MigrationStrategy.REPLACE_ENDPOINT, MigrationStrategy.REPLACE_REMOVED_API} +) +_VERSION_STRATEGIES = frozenset( + {MigrationStrategy.BUMP_PACKAGE_VERSION, MigrationStrategy.REFRESH_LOCKFILE, MigrationStrategy.VERIFY_UPGRADE} +) + + +def _requirement_id(*parts: str) -> str: + return hashlib.sha256("|".join(parts).encode()).hexdigest()[:16] + + +def _requirements_for_step(step: MigrationStep) -> tuple[VerificationRequirement, ...]: + out: list[VerificationRequirement] = [] + if step.strategy in _OBSOLETE_SYMBOL_STRATEGIES: + out.append( + VerificationRequirement( + requirement_id=_requirement_id("obsolete_symbol", step.step_id), + kind=VerificationRequirementKind.OBSOLETE_SYMBOL_REMOVED, + description=f"the obsolete usage {step.target.usage_token!r} no longer remains at {step.target.location}", + mandatory=True, + diff_item_keys=step.diff_item_keys, + usage_site_key=step.usage_site_key, + source_step_id=step.step_id, + ) + ) + if step.strategy in _CONTRACT_CALL_STRATEGIES: + out.append( + VerificationRequirement( + requirement_id=_requirement_id("sdk_call", step.step_id), + kind=VerificationRequirementKind.SDK_CALL_MATCHES_CONTRACT, + description=f"the modified call at {step.target.location} matches the new SDK surface", + mandatory=True, + diff_item_keys=step.diff_item_keys, + usage_site_key=step.usage_site_key, + source_step_id=step.step_id, + ) + ) + if step.strategy is MigrationStrategy.ADD_REQUIRED_PARAMETER: + out.append( + VerificationRequirement( + requirement_id=_requirement_id("required_arg", step.step_id), + kind=VerificationRequirementKind.REQUIRED_ARGUMENT_PRESENT, + description=f"the newly required argument is present at {step.target.location}", + mandatory=True, + diff_item_keys=step.diff_item_keys, + usage_site_key=step.usage_site_key, + source_step_id=step.step_id, + ) + ) + if step.strategy in _VERSION_STRATEGIES and step.version_constraint is not None: + out.append( + VerificationRequirement( + requirement_id=_requirement_id("version_constraint", step.step_id), + kind=VerificationRequirementKind.VERSION_CONSTRAINT_COMPATIBLE, + description=( + f"{step.version_constraint.manifest}: {step.version_constraint.package} constraint now " + f"allows {step.version_constraint.target}" + ), + mandatory=True, + diff_item_keys=step.diff_item_keys, + usage_site_key=step.usage_site_key, + source_step_id=step.step_id, + ) + ) + return tuple(out) + + +def generate_requirements( + plan: MigrationPlan, *, test_selections: tuple[TestSelection, ...] +) -> tuple[VerificationRequirement, ...]: + """Deterministic requirement generation from ``plan``'s own automatic + steps and the already-selected targeted tests. A plan with no + automatic steps at all (fully HUMAN_REQUIRED) yields no requirements + -- there is nothing PatchFrog itself changed to verify.""" + + automatic = plan.automatic_steps + if not automatic: + return () + + requirements: list[VerificationRequirement] = [ + VerificationRequirement( + requirement_id=_requirement_id("syntax", plan.change_fingerprint), + kind=VerificationRequirementKind.SYNTAX_VALID, + description="every file the patch modified remains syntactically valid", + mandatory=True, + ), + VerificationRequirement( + requirement_id=_requirement_id("imports", plan.change_fingerprint), + kind=VerificationRequirementKind.IMPORTS_RESOLVE, + description=( + "every modified module still imports cleanly (best-effort: PatchFrog's own sandbox may " + "lack the target repository's third-party dependencies, so a failure here is evidence, " + "not by itself proof of a broken migration -- never mandatory alone for VERIFIED)" + ), + mandatory=False, + ), + VerificationRequirement( + requirement_id=_requirement_id("typecheck", plan.change_fingerprint), + kind=VerificationRequirementKind.TYPE_CHECK_PASSES, + description="static type check of the modified files (best-effort; never mandatory alone for VERIFIED)", + mandatory=False, + ), + ] + + for step in automatic: + requirements.extend(_requirements_for_step(step)) + + contract_requirement_ids = tuple( + r.requirement_id + for r in requirements + if r.kind + in ( + VerificationRequirementKind.OBSOLETE_SYMBOL_REMOVED, + VerificationRequirementKind.SDK_CALL_MATCHES_CONTRACT, + VerificationRequirementKind.REQUIRED_ARGUMENT_PRESENT, + VerificationRequirementKind.VERSION_CONSTRAINT_COMPATIBLE, + ) + ) + if contract_requirement_ids: + requirements.append( + VerificationRequirement( + requirement_id=_requirement_id("contract_restored", plan.change_fingerprint), + kind=VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED, + description="the upstream contract change this migration targets is fully addressed", + mandatory=True, + diff_item_keys=tuple(sorted({k for r in requirements for k in r.diff_item_keys})), + ) + ) + + requirements.append( + VerificationRequirement( + requirement_id=_requirement_id("unit_tests", plan.change_fingerprint), + kind=VerificationRequirementKind.UNIT_TESTS_PASS, + description="targeted unit tests covering the affected consumers pass on the patched state", + #: Mandatory whenever real code changed -- its status becomes + #: UNAVAILABLE (never guessed PASSED) when no test evidence + #: exists at all, which the decision engine treats as + #: insufficient evidence for VERIFIED, not as a failure. + mandatory=True, + ) + ) + requirements.append( + VerificationRequirement( + requirement_id=_requirement_id("consumer_behavior", plan.change_fingerprint), + kind=VerificationRequirementKind.CONSUMER_BEHAVIOR_PRESERVED, + description="downstream consumer behavior is preserved, where baseline-vs-patched evidence exists", + mandatory=False, + ) + ) + + return tuple(requirements) + + +__all__ = ["generate_requirements"] diff --git a/patchfrog/migration_verification/service.py b/patchfrog/migration_verification/service.py new file mode 100644 index 0000000..02d88b0 --- /dev/null +++ b/patchfrog/migration_verification/service.py @@ -0,0 +1,295 @@ +"""Top-level Migration Verification orchestrator (M8). + + migration plan + generated patch + exact repository state + -> requirements (M8.2) -> targeted test selection (M8.3) + -> bounded execution plan (M8.4) -> sandboxed execution (M8.5) + -> baseline-vs-patched comparison (M8.6) -> contract verification (M8.7) + -> regression detection (M8.8) -> evidence bundle (M8.9/M8.10) + -> deterministic decision (M8.11) + +Never calls a provider. The one entry point, +:func:`run_migration_verification`, is safe to call for any +:class:`~patchfrog.migration.domain.MigrationResult`, including one with +no patch at all (returns an evidence-free ``UNVERIFIED``/``HUMAN_REQUIRED`` +bundle immediately, no sandbox touched). +""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path + +from patchfrog.executable_verification.domain import ExecutableVerificationEvidence +from patchfrog.executable_verification.pytest_adapter import run_pytest_verification +from patchfrog.executable_verification.sandbox import VerificationSandbox, is_sandbox_available +from patchfrog.migration.domain import GeneratedPatch, MigrationPlan +from patchfrog.migration_verification import baseline as baseline_mod +from patchfrog.migration_verification import contract as contract_mod +from patchfrog.migration_verification import evidence as evidence_mod +from patchfrog.migration_verification import execution_plan as execution_plan_mod +from patchfrog.migration_verification import requirements as requirements_mod +from patchfrog.migration_verification import test_selection as test_selection_mod +from patchfrog.migration_verification.adapters.imports import module_name_for_path, run_import_check +from patchfrog.migration_verification.adapters.mypy_adapter import run_type_check +from patchfrog.migration_verification.adapters.syntax import run_syntax_check +from patchfrog.migration_verification.decision import decide_outcome +from patchfrog.migration_verification.domain import ( + MAX_STEP_STDERR_EXCERPT_BYTES, + MAX_STEP_STDOUT_EXCERPT_BYTES, + BaselineComparison, + CheckStatus, + EvidenceStrength, + MigrationEvidenceBundle, + MigrationVerificationPlan, + StepEvidence, + TestSelectionClass, + VerificationCoverage, + VerificationRequirementKind, + VerificationStepKind, +) +from patchfrog.migration_verification.regression import detect_regressions +from patchfrog.upstream.blast_radius import BlastRadius + +#: Never run baseline comparison for every selected test -- bounded, +#: policy-controlled (M8.6's own instruction), and only for the +#: highest-confidence (DIRECT) selections. +MAX_BASELINE_COMPARISONS = 2 + +_EXEC_TO_CHECK_STATUS = { + "passed": CheckStatus.PASSED, + "confirmed_failure": CheckStatus.FAILED, + "timeout": CheckStatus.UNAVAILABLE, + "unsupported": CheckStatus.UNAVAILABLE, + "inconclusive": CheckStatus.UNAVAILABLE, + "sandbox_error": CheckStatus.UNAVAILABLE, +} + + +def _bounded(text: str, limit: int) -> str: + encoded = text.encode("utf-8") + if len(encoded) <= limit: + return text + return encoded[:limit].decode("utf-8", errors="ignore") + "... (truncated)" + + +def _pytest_evidence_to_step( + evidence: ExecutableVerificationEvidence, *, step_id: str, requirement_ids: tuple[str, ...] +) -> StepEvidence: + return StepEvidence( + step_id=step_id, + kind=VerificationStepKind.TARGETED_UNIT_TEST, + requirement_ids=requirement_ids, + status=_EXEC_TO_CHECK_STATUS[evidence.outcome.value], + exit_code=evidence.exit_code, + duration_ms=evidence.duration_ms, + stdout_excerpt=_bounded(evidence.stdout_excerpt, MAX_STEP_STDOUT_EXCERPT_BYTES), + stderr_excerpt=_bounded(evidence.stderr_excerpt, MAX_STEP_STDERR_EXCERPT_BYTES), + detail=f"pytest outcome: {evidence.outcome.value}", + ) + + +def _sandbox_unavailable_evidence(step_id: str, kind: VerificationStepKind, requirement_ids: tuple[str, ...]) -> StepEvidence: + return StepEvidence( + step_id=step_id, kind=kind, requirement_ids=requirement_ids, status=CheckStatus.UNAVAILABLE, exit_code=None, + duration_ms=0.0, stdout_excerpt="", stderr_excerpt="", + detail="verification sandbox unavailable on this host -- see docs/executable-verification.md", + ) + + +async def run_migration_verification( + *, + plan: MigrationPlan, + patch: GeneratedPatch | None, + blast_radii: tuple[BlastRadius, ...], + root: Path, + commit_sha: str, + extra_workspace_paths: tuple[Path, ...] = (), +) -> MigrationEvidenceBundle: + """``extra_workspace_paths``: directories merged into the disposable + verification workspace *only* -- never into ``root`` itself, and + never seen by dependency discovery/planning (M6/M7). Exists solely so + a bundled demo/test fixture can supply a runnable stand-in for a + third-party SDK the sandbox would otherwise lack, without that + stand-in changing what M6/M7 detect as an external dependency usage + site. Empty by default; real repository verification never needs it + (the target repository's own dependencies are either present or + genuinely unavailable, which is itself honest evidence -- see + :data:`patchfrog.migration_verification.domain.CheckStatus.UNAVAILABLE`).""" + + if patch is None: + empty_plan = MigrationVerificationPlan( + change_fingerprint=plan.change_fingerprint, patch_fingerprint="", requirements=(), steps=(), + test_selections=(), + ) + coverage = evidence_mod.build_coverage(empty_plan, step_evidence=(), contract_checks=()) + outcome, reasons = decide_outcome( + empty_plan, coverage=coverage, evidence_strength=_no_evidence(), regressions=(), + has_unresolved_human_steps=bool(plan.unresolved_steps), + ) + return evidence_mod.build_evidence_bundle( + plan=empty_plan, repository=plan.repository, repository_head_sha=commit_sha or "", + step_evidence=(), contract_checks=(), baseline_comparisons=(), coverage=coverage, + evidence_strength=_no_evidence(), residual_risk=plan.residual_risk, outcome=outcome, + outcome_reasons=reasons, + ) + + test_selections = test_selection_mod.select_targeted_tests(plan, blast_radii) + requirements = requirements_mod.generate_requirements(plan, test_selections=test_selections) + steps = execution_plan_mod.build_execution_plan(requirements, patch=patch, test_selections=test_selections) + verification_plan = MigrationVerificationPlan( + change_fingerprint=plan.change_fingerprint, patch_fingerprint=patch.fingerprint, + requirements=requirements, steps=steps, test_selections=test_selections, + ) + + contract_checks = contract_mod.verify_contract(plan, patch, requirements) + + if not requirements: + coverage = evidence_mod.build_coverage(verification_plan, step_evidence=(), contract_checks=contract_checks) + outcome, reasons = decide_outcome( + verification_plan, coverage=coverage, evidence_strength=_no_evidence(), regressions=(), + has_unresolved_human_steps=bool(plan.unresolved_steps), + ) + return evidence_mod.build_evidence_bundle( + plan=verification_plan, repository=plan.repository, repository_head_sha=commit_sha or "", + step_evidence=(), contract_checks=contract_checks, baseline_comparisons=(), coverage=coverage, + evidence_strength=_no_evidence(), residual_risk=plan.residual_risk, outcome=outcome, outcome_reasons=reasons, + ) + + sandbox_available = is_sandbox_available() + step_evidence: list[StepEvidence] = [] + baseline_comparisons: list[BaselineComparison] = [] + + if not sandbox_available: + for step in steps: + step_evidence.append(_sandbox_unavailable_evidence(step.step_id, step.kind, step.requirement_ids)) + else: + sandbox = VerificationSandbox(timeout_seconds=steps[0].timeout_seconds if steps else 30.0) + workspace = Path(tempfile.mkdtemp(prefix="patchfrog-migration-verify-")) + try: + shutil.copytree(root, workspace, dirs_exist_ok=True, symlinks=True) + for extra_path in extra_workspace_paths: + shutil.copytree(extra_path, workspace, dirs_exist_ok=True, symlinks=True) + for relative_path, content in patch.new_contents.items(): + target = workspace / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + + python_files = tuple(sorted(f for f in patch.modified_files if f.endswith(".py"))) + for step in steps: + if step.kind is VerificationStepKind.SYNTAX_CHECK: + step_evidence.append( + await run_syntax_check( + sandbox, workspace_root=workspace, step_id=step.step_id, + requirement_ids=step.requirement_ids, file_paths=python_files, + ) + ) + elif step.kind is VerificationStepKind.IMPORT_CHECK: + module_names = tuple(sorted({m for f in python_files if (m := module_name_for_path(f)) is not None})) + step_evidence.append( + await run_import_check( + sandbox, workspace_root=workspace, step_id=step.step_id, + requirement_ids=step.requirement_ids, module_names=module_names, + ) + ) + elif step.kind is VerificationStepKind.TYPE_CHECK: + step_evidence.append( + await run_type_check( + sandbox, workspace_root=workspace, step_id=step.step_id, + requirement_ids=step.requirement_ids, file_paths=python_files, + ) + ) + elif step.kind is VerificationStepKind.TARGETED_UNIT_TEST: + test_path = step.command[-1] + pytest_evidence = await run_pytest_verification( + sandbox, workspace_root=workspace, test_target_path=test_path, commit_sha=commit_sha, + ) + step_evidence.append( + _pytest_evidence_to_step(pytest_evidence, step_id=step.step_id, requirement_ids=step.requirement_ids) + ) + + consumer_behavior_req = next( + (r for r in requirements if r.kind is VerificationRequirementKind.CONSUMER_BEHAVIOR_PRESERVED), None + ) + if consumer_behavior_req is not None: + # DIRECT and TRANSITIVE only -- FALLBACK/UNKNOWN selections + # are heuristic-strength at best (M8.3) and never trusted + # enough to spend a baseline run on. + strong_selections = [ + s for s in test_selections + if s.classification in (TestSelectionClass.DIRECT, TestSelectionClass.TRANSITIVE) + ] + for selection in strong_selections[:MAX_BASELINE_COMPARISONS]: + baseline_comparisons.append( + await baseline_mod.run_baseline_comparison( + sandbox, root=root, patch=patch, requirement_id=consumer_behavior_req.requirement_id, + test_path=selection.test_path, commit_sha=commit_sha, + extra_workspace_paths=extra_workspace_paths, + ) + ) + finally: + shutil.rmtree(workspace, ignore_errors=True) + + step_evidence_t = tuple(step_evidence) + baseline_comparisons_t = tuple(baseline_comparisons) + coverage = evidence_mod.build_coverage(verification_plan, step_evidence=step_evidence_t, contract_checks=contract_checks) + coverage = _merge_baseline_into_coverage(coverage, baseline_comparisons_t) + evidence_strength = evidence_mod.compute_evidence_strength( + step_evidence=step_evidence_t, contract_checks=contract_checks, baseline_comparisons=baseline_comparisons_t, + ) + regressions = detect_regressions( + plan, patch, step_evidence=step_evidence_t, contract_checks=contract_checks, + baseline_comparisons=baseline_comparisons_t, + ) + has_unresolved_human_steps = bool(plan.unresolved_steps) + outcome, reasons = decide_outcome( + verification_plan, coverage=coverage, evidence_strength=evidence_strength, regressions=regressions, + has_unresolved_human_steps=has_unresolved_human_steps, + ) + residual_risk = evidence_mod.compute_residual_risk( + coverage, evidence_strength=evidence_strength, has_unresolved_human_steps=has_unresolved_human_steps, + ) + return evidence_mod.build_evidence_bundle( + plan=verification_plan, repository=plan.repository, repository_head_sha=commit_sha or "", + step_evidence=step_evidence_t, contract_checks=contract_checks, baseline_comparisons=baseline_comparisons_t, + coverage=coverage, evidence_strength=evidence_strength, residual_risk=residual_risk, outcome=outcome, + outcome_reasons=reasons, + ) + + +def _no_evidence() -> EvidenceStrength: + return EvidenceStrength.NONE + + +def _merge_baseline_into_coverage( + coverage: VerificationCoverage, baseline_comparisons: tuple[BaselineComparison, ...], +) -> VerificationCoverage: + if not baseline_comparisons: + return coverage + + satisfied = set(coverage.satisfied_requirement_ids) + failed = set(coverage.failed_requirement_ids) + not_run = set(coverage.not_run_requirement_ids) + unavailable = set(coverage.unavailable_requirement_ids) + for comparison in baseline_comparisons: + req_id = comparison.requirement_id + not_run.discard(req_id) + unavailable.discard(req_id) + if comparison.patched_status is CheckStatus.PASSED: + satisfied.add(req_id) + failed.discard(req_id) + elif comparison.patched_status is CheckStatus.FAILED: + failed.add(req_id) + satisfied.discard(req_id) + else: + unavailable.add(req_id) + return VerificationCoverage( + mandatory_requirement_ids=coverage.mandatory_requirement_ids, + satisfied_requirement_ids=tuple(sorted(satisfied)), + failed_requirement_ids=tuple(sorted(failed)), + not_run_requirement_ids=tuple(sorted(not_run)), + unavailable_requirement_ids=tuple(sorted(unavailable)), + ) + + +__all__ = ["MAX_BASELINE_COMPARISONS", "run_migration_verification"] diff --git a/patchfrog/migration_verification/store.py b/patchfrog/migration_verification/store.py new file mode 100644 index 0000000..bc18237 --- /dev/null +++ b/patchfrog/migration_verification/store.py @@ -0,0 +1,107 @@ +"""Idempotent persistence of migration verification evidence bundles (M8.9). + +Mirrors :mod:`patchfrog.migration.store`'s pattern exactly: one row per +(patch, verification-plan fingerprint); re-verifying the identical patch +against the identical requirement/step derivation only refreshes +``last_verified_at``. A different patch fingerprint or a different +verification-plan fingerprint (the M8 engine's own logic changed) never +matches an existing row -- old evidence is invalidated by construction, +never silently reused for a new patch (M9.8's integrity requirement +starts here). +""" + +from __future__ import annotations + +import hashlib +import json +import uuid +from datetime import UTC, datetime + +from sqlalchemy import select, text +from sqlalchemy.ext.asyncio import AsyncSession + +from patchfrog.migration_verification.domain import MigrationEvidenceBundle +from patchfrog.migration_verification.report import bundle_to_dict +from patchfrog.persistence.models.migration_verification import MigrationVerificationRunModel + +MAX_JSON_COLUMN_BYTES = 256_000 + + +def _bounded(value: object, limit: int = MAX_JSON_COLUMN_BYTES) -> str: + rendered = json.dumps(value, sort_keys=True, separators=(",", ":"), default=str) + if len(rendered.encode()) <= limit: + return rendered + return json.dumps({"truncated": True, "bytes": len(rendered.encode())}) + + +async def _advisory_lock(session: AsyncSession, key: str) -> None: + if session.bind is None or session.bind.dialect.name != "postgresql": + return + digest = hashlib.sha256(key.encode()).digest()[:8] + await session.execute( + text("SELECT pg_advisory_xact_lock(:key)"), {"key": int.from_bytes(digest, "big") & 0x7FFFFFFFFFFFFFFF} + ) + + +class MigrationVerificationStore: + async def record_run( + self, + session: AsyncSession, + *, + patch_id: uuid.UUID, + bundle: MigrationEvidenceBundle, + recorded_at: datetime | None = None, + ) -> tuple[MigrationVerificationRunModel, bool]: + """Returns (row, created).""" + + now = recorded_at or datetime.now(UTC) + plan_fingerprint = bundle.plan.fingerprint() + await _advisory_lock(session, f"migration-verification:{patch_id}:{plan_fingerprint}") + existing = ( + await session.execute( + select(MigrationVerificationRunModel).where( + MigrationVerificationRunModel.patch_id == patch_id, + MigrationVerificationRunModel.verification_plan_fingerprint == plan_fingerprint, + ) + ) + ).scalar_one_or_none() + if existing is not None: + existing.last_verified_at = now + existing.outcome = bundle.outcome.value + existing.evidence_strength = bundle.evidence_strength.value + existing.residual_risk = bundle.residual_risk.value + existing.bundle_json = _bounded(bundle_to_dict(bundle)) + await session.flush() + return existing, False + + row = MigrationVerificationRunModel( + patch_id=patch_id, + change_fingerprint=bundle.change_fingerprint, + patch_fingerprint=bundle.patch_fingerprint, + verification_plan_fingerprint=plan_fingerprint, + engine_version=bundle.version, + repository_head_sha=bundle.repository_head_sha, + outcome=bundle.outcome.value, + evidence_strength=bundle.evidence_strength.value, + residual_risk=bundle.residual_risk.value, + bundle_json=_bounded(bundle_to_dict(bundle)), + created_at=now, + last_verified_at=now, + ) + session.add(row) + await session.flush() + return row, True + + async def latest_for_patch( + self, session: AsyncSession, *, patch_id: uuid.UUID + ) -> MigrationVerificationRunModel | None: + query = ( + select(MigrationVerificationRunModel) + .where(MigrationVerificationRunModel.patch_id == patch_id) + .order_by(MigrationVerificationRunModel.last_verified_at.desc()) + .limit(1) + ) + return (await session.execute(query)).scalar_one_or_none() + + +__all__ = ["MAX_JSON_COLUMN_BYTES", "MigrationVerificationStore"] diff --git a/patchfrog/migration_verification/test_selection.py b/patchfrog/migration_verification/test_selection.py new file mode 100644 index 0000000..d74879c --- /dev/null +++ b/patchfrog/migration_verification/test_selection.py @@ -0,0 +1,81 @@ +"""Deterministic targeted-test selection for migration verification (M8.3). + +Reuses :class:`patchfrog.upstream.blast_radius.BlastRadius` -- already +computed by M6/M7 for every migration plan -- rather than re-deriving a +second test-to-symbol mapping. ``BlastRadius.related_tests`` already +carries a confidence and a reason per test node; this module only +translates that existing evidence into the bounded +:class:`~patchfrog.migration_verification.domain.TestSelection` taxonomy +M8.3 asks for, and explains why every selection was made. + +Never a filename-similarity guess (the same discipline +:mod:`patchfrog.executable_verification.eligibility` already applies for +review candidates): a file with no test evidence in the blast radius at +all is ``UNKNOWN``, never given an invented test path. +""" + +from __future__ import annotations + +from patchfrog.dependencies.domain import DetectionConfidence +from patchfrog.migration.domain import MigrationPlan +from patchfrog.migration_verification.domain import ( + MAX_TARGETED_TESTS_PER_PLAN, + TestSelection, + TestSelectionClass, +) +from patchfrog.upstream.blast_radius import BlastRadius + + +def _classify(confidence: DetectionConfidence, *, is_direct_site: bool) -> TestSelectionClass: + if is_direct_site and confidence is DetectionConfidence.HIGH: + return TestSelectionClass.DIRECT + if confidence in (DetectionConfidence.HIGH, DetectionConfidence.MEDIUM): + return TestSelectionClass.TRANSITIVE + return TestSelectionClass.FALLBACK + + +def select_targeted_tests( + plan: MigrationPlan, blast_radii: tuple[BlastRadius, ...] +) -> tuple[TestSelection, ...]: + """One :class:`TestSelection` per distinct related-test file across + every :class:`BlastRadius` the migration's dependency touches, + classified DIRECT/TRANSITIVE/FALLBACK by the same confidence the + blast radius already computed. Bounded to + ``MAX_TARGETED_TESTS_PER_PLAN`` -- the highest-confidence selections + are kept first (never "run everything" by default, M8.2's own rule). + + A plan with automatic steps but zero related-test evidence anywhere + in its blast radii yields no selections at all -- the caller (see + :mod:`patchfrog.migration_verification.requirements`) must then treat + ``UNIT_TESTS_PASS`` as a requirement with no discoverable evidence, + never invent one.""" + + if not plan.automatic_steps: + return () + + seen: dict[str, TestSelection] = {} + for radius in blast_radii: + direct_files = {n.file_path for n in radius.direct} + for related in radius.related_tests: + if related.file_path in seen: + continue + classification = _classify(related.confidence, is_direct_site=related.covers in direct_files or any( + related.covers.startswith(f) for f in direct_files + )) + seen[related.file_path] = TestSelection( + test_path=related.file_path, + classification=classification, + reason=f"{related.reason} (confidence={related.confidence.value})", + ) + + ordered = sorted( + seen.values(), + key=lambda s: ( + {"direct": 0, "transitive": 1, "fallback": 2, "unknown": 3}[s.classification.value], + s.test_path, + ), + ) + return tuple(ordered[:MAX_TARGETED_TESTS_PER_PLAN]) + + +__all__ = ["select_targeted_tests"] diff --git a/patchfrog/persistence/models/__init__.py b/patchfrog/persistence/models/__init__.py index fe43166..21d2a1b 100644 --- a/patchfrog/persistence/models/__init__.py +++ b/patchfrog/persistence/models/__init__.py @@ -39,6 +39,8 @@ InstallationStatus, ) from patchfrog.persistence.models.learning import RepositoryLearningRecordModel +from patchfrog.persistence.models.migration_pr import MigrationPRModel +from patchfrog.persistence.models.migration_verification import MigrationVerificationRunModel from patchfrog.persistence.models.parsed_file_cache import ParsedFileCacheModel from patchfrog.persistence.models.publishing import ( ReviewPublicationCommentModel, @@ -104,8 +106,10 @@ "IngestionStatus", "InstallationModel", "InstallationStatus", + "MigrationPRModel", "MigrationPatchModel", "MigrationPlanModel", + "MigrationVerificationRunModel", "ParsedFileCacheModel", "PullRequestIngestionModel", "PullRequestModel", diff --git a/patchfrog/persistence/models/migration_pr.py b/patchfrog/persistence/models/migration_pr.py new file mode 100644 index 0000000..957ef6e --- /dev/null +++ b/patchfrog/persistence/models/migration_pr.py @@ -0,0 +1,51 @@ +"""Migration pull-request publication persistence (M9.1/M9.6). + +One bounded table, keyed by the migration's stable identity (repository + +upstream change fingerprint + engine version -- see +:meth:`patchfrog.migration_pr.domain.MigrationPRLinkage.identity_key`), +never by patch/evidence fingerprint: the whole point is that a +*regenerated* patch for the same upstream change reconciles the same row +(and the same GitHub PR) rather than creating a duplicate (M9.6). +""" + +from __future__ import annotations + +import uuid +from datetime import datetime + +from sqlalchemy import DateTime, Index, Integer, String, Text, UniqueConstraint +from sqlalchemy.orm import Mapped, mapped_column +from sqlalchemy.types import Uuid + +from patchfrog.persistence.models.base import Base + + +class MigrationPRModel(Base): + __tablename__ = "migration_pull_requests" + __table_args__ = ( + UniqueConstraint("identity_key", name="uq_migration_pull_requests_identity"), + Index("ix_migration_pull_requests_repository", "repository"), + ) + + id: Mapped[uuid.UUID] = mapped_column(Uuid, primary_key=True, default=uuid.uuid4) + identity_key: Mapped[str] = mapped_column(String(64)) + repository: Mapped[str] = mapped_column(String(512)) + change_fingerprint: Mapped[str] = mapped_column(String(64)) + engine_version: Mapped[int] = mapped_column(Integer) + branch_name: Mapped[str] = mapped_column(String(255)) + patch_fingerprint: Mapped[str] = mapped_column(String(64)) + verification_plan_fingerprint: Mapped[str] = mapped_column(String(64)) + bundle_fingerprint: Mapped[str] = mapped_column(String(64)) + base_commit_sha: Mapped[str] = mapped_column(String(64)) + #: MigrationPREligibility value. + eligibility: Mapped[str] = mapped_column(String(32)) + #: MigrationPRStatus value. + status: Mapped[str] = mapped_column(String(32)) + pr_number: Mapped[int | None] = mapped_column(Integer, nullable=True) + pr_html_url: Mapped[str | None] = mapped_column(String(1024), nullable=True) + reason: Mapped[str | None] = mapped_column(Text, nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + last_reconciled_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + + +__all__ = ["MigrationPRModel"] diff --git a/patchfrog/persistence/models/migration_verification.py b/patchfrog/persistence/models/migration_verification.py new file mode 100644 index 0000000..05aed90 --- /dev/null +++ b/patchfrog/persistence/models/migration_verification.py @@ -0,0 +1,50 @@ +"""Migration Verification run persistence (M8.9's evidence bundle). + +One bounded table. Mirrors :mod:`patchfrog.persistence.models.upstream`'s +``migration_patches`` pattern exactly: one row per (patch, verification +plan fingerprint) -- a re-run against the identical patch and requirement +set is idempotent (only ``last_verified_at`` moves); a different patch +fingerprint (the migration changed) or a different verification-plan +fingerprint (the M8 engine's own requirement/step derivation changed) is +never matched against a stale row -- old evidence is never silently +reused for a new patch. +""" + +from __future__ import annotations + +import uuid +from datetime import datetime + +from sqlalchemy import DateTime, ForeignKey, Index, Integer, String, Text, UniqueConstraint +from sqlalchemy.orm import Mapped, mapped_column +from sqlalchemy.types import Uuid + +from patchfrog.persistence.models.base import Base + + +class MigrationVerificationRunModel(Base): + __tablename__ = "migration_verification_runs" + __table_args__ = ( + UniqueConstraint( + "patch_id", "verification_plan_fingerprint", name="uq_migration_verification_runs_identity" + ), + Index("ix_migration_verification_runs_patch", "patch_id"), + ) + + id: Mapped[uuid.UUID] = mapped_column(Uuid, primary_key=True, default=uuid.uuid4) + patch_id: Mapped[uuid.UUID] = mapped_column(Uuid, ForeignKey("migration_patches.id", ondelete="CASCADE")) + change_fingerprint: Mapped[str] = mapped_column(String(64)) + patch_fingerprint: Mapped[str] = mapped_column(String(64)) + verification_plan_fingerprint: Mapped[str] = mapped_column(String(64)) + engine_version: Mapped[int] = mapped_column(Integer) + repository_head_sha: Mapped[str] = mapped_column(String(64), default="") + #: VerificationOutcome value. + outcome: Mapped[str] = mapped_column(String(24)) + evidence_strength: Mapped[str] = mapped_column(String(16)) + residual_risk: Mapped[str] = mapped_column(String(16)) + bundle_json: Mapped[str] = mapped_column(Text, default="{}") + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + last_verified_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + + +__all__ = ["MigrationVerificationRunModel"] diff --git a/patchfrog/publishing/checks.py b/patchfrog/publishing/checks.py index 4d7f663..2f23653 100644 --- a/patchfrog/publishing/checks.py +++ b/patchfrog/publishing/checks.py @@ -45,7 +45,9 @@ class ReviewCheckUpdate: class CheckRunClient(Protocol): - async def list_check_runs(self, *, installation_id: int, ref: PullRequestRef, head_sha: str) -> list[GitHubCheckRun]: ... + async def list_check_runs( + self, *, installation_id: int, ref: PullRequestRef, head_sha: str, check_name: str = "PatchFrog review" + ) -> list[GitHubCheckRun]: ... async def create_check_run( self, *, installation_id: int, ref: PullRequestRef, check: GitHubCheckRunInput diff --git a/tests/fixtures/upstream_changes/demo/README.md b/tests/fixtures/upstream_changes/demo/README.md index 648163c..dfd8b0f 100644 --- a/tests/fixtures/upstream_changes/demo/README.md +++ b/tests/fixtures/upstream_changes/demo/README.md @@ -5,3 +5,11 @@ old contract `client.chat.create(prompt=...)` -> new contract `client.responses.create(input=...)`. No real vendor API is described. python -m patchfrog.cli migrations demo + +This fixture has one consumer usage the migration cannot fully automate +(a removed `stream` argument), so the full pipeline -- extended through +M8 verification and the M9 dry-run PR dossier -- always ends in +`HUMAN_REQUIRED` / `PLAN_ONLY` here. See the sibling `demo_verified/` +fixture (`python -m patchfrog.cli migrations demo --verified`) for the +fully-automatic counterpart, which reaches `VERIFIED` / `AUTO_OPEN` with +real baseline-vs-patched evidence. diff --git a/tests/fixtures/upstream_changes/demo/sdk_stub/acme_ai/__init__.py b/tests/fixtures/upstream_changes/demo/sdk_stub/acme_ai/__init__.py new file mode 100644 index 0000000..1350f49 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo/sdk_stub/acme_ai/__init__.py @@ -0,0 +1,50 @@ +"""A tiny, fully local, fictional stand-in for the "acme-ai" SDK's 2.0 +surface -- given only to Migration *Verification* (M8), never to +dependency discovery/planning, so it cannot change what M6/M7 detect as +an external dependency. See +:data:`patchfrog.cli_changes.DEMO_SDK_STUB_ROOT`. + +Deliberately implements only the 2.0 (``responses.create``) shape, never +``chat.create`` -- this simulates the environment already having the new +SDK installed. Code still written against the 1.4 surface +(``client.chat.create(...)``) genuinely raises ``AttributeError`` here, +exactly like the real upstream removal it stands in for; migrated code +(``client.responses.create(...)``) genuinely runs and returns a value. +This is what lets the M8 baseline-vs-patched comparison observe a real +``baseline_fail_patched_pass`` result instead of only a syntactic one. +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True, slots=True) +class Response: + output_text: str + id: str + + +@dataclass(frozen=True, slots=True) +class Embedding: + id: str + vector: tuple[float, ...] + + +class _Responses: + def create(self, *, model: str, input: str, temperature: float | None = None) -> Response: + return Response(output_text=f"echo: {input}", id="resp_demo_1") + + +class _Embeddings: + def create(self, *, model: str, input: str) -> Embedding: + return Embedding(id="emb_demo_1", vector=(0.0, 0.0, 0.0)) + + +class Client: + def __init__(self) -> None: + self.responses = _Responses() + self.embeddings = _Embeddings() + + +__all__ = ["Client", "Embedding", "Response"] diff --git a/tests/fixtures/upstream_changes/demo_verified/hints.yaml b/tests/fixtures/upstream_changes/demo_verified/hints.yaml new file mode 100644 index 0000000..4cf3b4e --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/hints.yaml @@ -0,0 +1,8 @@ +patchfrog_change_hints: 1 +provenance: "acme-ai 2.0 migration guide (fictional demo SDK)" +symbol_renames: + - {old: chat.create, new: responses.create} +parameter_renames: + - {symbol: chat.create, old: prompt, new: input} +return_field_renames: + - {symbol: chat.create, old: text, new: output_text} diff --git a/tests/fixtures/upstream_changes/demo_verified/new.yaml b/tests/fixtures/upstream_changes/demo_verified/new.yaml new file mode 100644 index 0000000..e0c94aa --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/new.yaml @@ -0,0 +1,18 @@ +patchfrog_sdk_surface: 1 +package: acme-ai +ecosystem: pypi +version: "2.0.0" +modules: [acme_ai] +symbols: + Client: {} + responses.create: + params: + model: {required: true, type: string} + input: {required: true, type: string} + temperature: {type: number} + returns: + fields: {output_text: string, id: string} + embeddings.create: + params: + model: {required: true, type: string} + input: {required: true, type: string} diff --git a/tests/fixtures/upstream_changes/demo_verified/old.yaml b/tests/fixtures/upstream_changes/demo_verified/old.yaml new file mode 100644 index 0000000..bd92c02 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/old.yaml @@ -0,0 +1,19 @@ +patchfrog_sdk_surface: 1 +package: acme-ai +ecosystem: pypi +version: "1.4.0" +modules: [acme_ai] +symbols: + Client: {} + chat.create: + params: + model: {required: true, type: string} + prompt: {required: true, type: string} + stream: {type: boolean} + temperature: {type: number} + returns: + fields: {text: string, id: string} + embeddings.create: + params: + model: {required: true, type: string} + input: {required: true, type: string} diff --git a/tests/fixtures/upstream_changes/demo_verified/repo/app/__init__.py b/tests/fixtures/upstream_changes/demo_verified/repo/app/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/upstream_changes/demo_verified/repo/app/ai/__init__.py b/tests/fixtures/upstream_changes/demo_verified/repo/app/ai/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/upstream_changes/demo_verified/repo/app/ai/chat.py b/tests/fixtures/upstream_changes/demo_verified/repo/app/ai/chat.py new file mode 100644 index 0000000..201f9f1 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/repo/app/ai/chat.py @@ -0,0 +1,10 @@ +"""Chat replies for the support widget.""" + +import acme_ai + +client = acme_ai.Client() + + +def generate_reply(message: str) -> str: + result = client.chat.create(model="acme-large", prompt=message, temperature=0.2) + return result.text diff --git a/tests/fixtures/upstream_changes/demo_verified/repo/requirements.txt b/tests/fixtures/upstream_changes/demo_verified/repo/requirements.txt new file mode 100644 index 0000000..201c758 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/repo/requirements.txt @@ -0,0 +1,2 @@ +acme-ai==1.4.0 +httpx==0.27.0 diff --git a/tests/fixtures/upstream_changes/demo_verified/repo/tests/test_chat.py b/tests/fixtures/upstream_changes/demo_verified/repo/tests/test_chat.py new file mode 100644 index 0000000..2634d83 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/repo/tests/test_chat.py @@ -0,0 +1,5 @@ +from app.ai.chat import generate_reply + + +def test_generate_reply_is_text() -> None: + assert isinstance(generate_reply("hi"), str) diff --git a/tests/fixtures/upstream_changes/demo_verified/sdk_stub/acme_ai/__init__.py b/tests/fixtures/upstream_changes/demo_verified/sdk_stub/acme_ai/__init__.py new file mode 100644 index 0000000..1350f49 --- /dev/null +++ b/tests/fixtures/upstream_changes/demo_verified/sdk_stub/acme_ai/__init__.py @@ -0,0 +1,50 @@ +"""A tiny, fully local, fictional stand-in for the "acme-ai" SDK's 2.0 +surface -- given only to Migration *Verification* (M8), never to +dependency discovery/planning, so it cannot change what M6/M7 detect as +an external dependency. See +:data:`patchfrog.cli_changes.DEMO_SDK_STUB_ROOT`. + +Deliberately implements only the 2.0 (``responses.create``) shape, never +``chat.create`` -- this simulates the environment already having the new +SDK installed. Code still written against the 1.4 surface +(``client.chat.create(...)``) genuinely raises ``AttributeError`` here, +exactly like the real upstream removal it stands in for; migrated code +(``client.responses.create(...)``) genuinely runs and returns a value. +This is what lets the M8 baseline-vs-patched comparison observe a real +``baseline_fail_patched_pass`` result instead of only a syntactic one. +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True, slots=True) +class Response: + output_text: str + id: str + + +@dataclass(frozen=True, slots=True) +class Embedding: + id: str + vector: tuple[float, ...] + + +class _Responses: + def create(self, *, model: str, input: str, temperature: float | None = None) -> Response: + return Response(output_text=f"echo: {input}", id="resp_demo_1") + + +class _Embeddings: + def create(self, *, model: str, input: str) -> Embedding: + return Embedding(id="emb_demo_1", vector=(0.0, 0.0, 0.0)) + + +class Client: + def __init__(self) -> None: + self.responses = _Responses() + self.embeddings = _Embeddings() + + +__all__ = ["Client", "Embedding", "Response"] diff --git a/tests/integration/test_migration_cli.py b/tests/integration/test_migration_cli.py index 3461173..b389640 100644 --- a/tests/integration/test_migration_cli.py +++ b/tests/integration/test_migration_cli.py @@ -15,6 +15,7 @@ from patchfrog.cli import main from patchfrog.config.settings import get_settings +from patchfrog.executable_verification.sandbox import is_sandbox_available from patchfrog.persistence.models.repository import RepositoryModel from tests.support.postgres import POSTGRES_TEST_URL, postgres_engine_or_skip from tests.support.upstream_cases import CASES_ROOT @@ -41,6 +42,36 @@ def test_migrations_demo_end_to_end(capsys: pytest.CaptureFixture[str]) -> None: assert payload["patch"]["origin"] == "deterministic" assert {"app/ai/chat.py", "workers/summary.py", "requirements.txt"} == set(payload["patch"]["modified_files"]) + # M9.12: the default demo also carries the migration through M8 + # verification and the M9 eligibility decision -- this fixture's one + # human-required step means it can never reach more than + # HUMAN_REQUIRED / PLAN_ONLY, however cleanly the rest checks out. + assert payload["verification"]["outcome"] == "human_required" + assert payload["pr_plan"]["eligibility"] == "plan_only" + assert payload["pr_plan"]["may_open_code_pr"] is False + + +@pytest.mark.skipif(not is_sandbox_available(), reason="verification sandbox unavailable on this host") +def test_migrations_demo_verified_end_to_end(capsys: pytest.CaptureFixture[str]) -> None: + """M9.12's full product demo: upstream change -> affected code -> + patch -> verification -> evidence-backed PR dossier, all the way to + VERIFIED and an AUTO_OPEN-eligible dry-run dossier.""" + + assert main(["migrations", "demo", "--verified"]) == 0 + text = capsys.readouterr().out + assert "Verification outcome: VERIFIED" in text + assert "Eligibility: AUTO_OPEN" in text + assert "Would open a code PR: yes" in text + assert "## Evidence identity" in text + + assert main(["migrations", "demo", "--verified", "--json"]) == 0 + payload = json.loads(capsys.readouterr().out) + assert payload["verification"]["outcome"] == "verified" + assert payload["verification"]["evidence_strength"] == "strong" + assert payload["pr_plan"]["eligibility"] == "auto_open" + assert payload["pr_plan"]["may_open_code_pr"] is True + assert payload["pr_plan"]["branch_name"].startswith("patchfrog/migrate/acme-ai/") + def test_migrations_plan_text_and_json(capsys: pytest.CaptureFixture[str]) -> None: argv = ["migrations", "plan", "--old", str(REQUIRED_PARAM / "old.yaml"), "--new", diff --git a/tests/integration/test_migration_pr_publisher.py b/tests/integration/test_migration_pr_publisher.py new file mode 100644 index 0000000..53c9320 --- /dev/null +++ b/tests/integration/test_migration_pr_publisher.py @@ -0,0 +1,206 @@ +"""M9.6 / M9.7 / M9.8 / M9.9: migration PR publication -- idempotency, +stale-base protection, eligibility gating, and check-run reconciliation, +all against :class:`FakeMigrationGitHubPublisher` (M9.9's own +requirement: no real GitHub PR is ever opened by an automated test). +""" + +from __future__ import annotations + +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from patchfrog.migration.domain import ( + GeneratedPatch, + PatchOrigin, + SafetyGateResult, + StepOutcome, + StepResult, +) +from patchfrog.migration_pr.domain import ( + MigrationPREligibility, + MigrationPRLinkage, + MigrationPRPlan, + MigrationPRPolicy, + MigrationPRStatus, +) +from patchfrog.migration_pr.eligibility import determine_eligibility +from patchfrog.migration_pr.fake_github import FakeMigrationGitHubPublisher +from patchfrog.migration_pr.publisher import MigrationPRPublicationMode, MigrationPRPublisher +from patchfrog.migration_verification.domain import VerificationOutcome + +BASE_SHA = "a" * 40 +OWNER, REPO = "octo", "widgets" +FULL_NAME = f"{OWNER}/{REPO}" + + +def _linkage(*, change_fingerprint: str = "c" * 64, patch_fingerprint: str = "p" * 64, base_sha: str = BASE_SHA) -> MigrationPRLinkage: + return MigrationPRLinkage( + change_fingerprint=change_fingerprint, dependency_key="acme-ai:pypi", repository=FULL_NAME, + base_commit_sha=base_sha, plan_fingerprint="pl" * 32, patch_fingerprint=patch_fingerprint, + verification_plan_fingerprint="v" * 64, bundle_fingerprint="e" * 64, + ) + + +def _plan(*, outcome: VerificationOutcome, policy: MigrationPRPolicy | None = None, linkage: MigrationPRLinkage | None = None) -> MigrationPRPlan: + linkage = linkage or _linkage() + eligibility, reason = determine_eligibility(outcome, policy=policy or MigrationPRPolicy()) + return MigrationPRPlan( + linkage=linkage, eligibility=eligibility, eligibility_reason=reason, + branch_name="patchfrog/migrate/acme-ai/" + linkage.change_fingerprint[:12], commit_message="migrate acme-ai", + pr_title="PatchFrog: migrate acme-ai", pr_body="dossier body", check_title="check title", + check_summary="check summary", + ) + + +def _patch(*, content: str = "new content") -> GeneratedPatch: + return GeneratedPatch( + origin=PatchOrigin.DETERMINISTIC, unified_diff="diff", modified_files=("app/chat.py",), + new_contents={"app/chat.py": content}, step_results=(StepResult("s1", StepOutcome.APPLIED, "d"),), + safety=(SafetyGateResult("g", True, "ok"),), + ) + + +def _publisher(session_factory: async_sessionmaker[AsyncSession], fake: FakeMigrationGitHubPublisher) -> MigrationPRPublisher: + return MigrationPRPublisher(session_factory=session_factory, publisher=fake, installation_id=1) + + +async def test_dry_run_makes_no_github_call(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.DRY_RUN) + assert result.status is MigrationPRStatus.DRY_RUN + assert fake.create_ref_calls == [] and fake.create_pull_request_calls == [] + + +async def test_verified_migration_opens_a_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.OPENED + assert result.number is not None + assert fake.create_ref_calls == [f"refs/heads/{plan.branch_name}"] + assert len(fake.checks) == 1 and fake.checks[0].conclusion is not None + + +async def test_republishing_identical_patch_is_idempotent_updates_same_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + publisher = _publisher(session_factory, fake) + first = await publisher.publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + second = await publisher.publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert first.number == second.number + assert second.status is MigrationPRStatus.UPDATED + assert len(fake.create_pull_request_calls) == 1 # never a duplicate PR + + +async def test_regenerated_patch_updates_the_same_branch_and_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + publisher = _publisher(session_factory, fake) + first = await publisher.publish(plan=plan, patch=_patch(content="v1"), mode=MigrationPRPublicationMode.PUBLISH) + second = await publisher.publish(plan=plan, patch=_patch(content="v2 -- different"), mode=MigrationPRPublicationMode.PUBLISH) + assert first.number == second.number + assert fake.update_ref_calls == [f"heads/{plan.branch_name}"] # force-moved, never a second create_ref + + +async def test_stale_base_never_publishes(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha="moved" + "0" * 35) + plan = _plan(outcome=VerificationOutcome.VERIFIED) # linkage.base_commit_sha is BASE_SHA, not the moved one + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.STALE_REQUIRES_REGENERATION + assert fake.create_ref_calls == [] and fake.create_pull_request_calls == [] + + +async def test_failed_outcome_never_opens_a_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.FAILED) + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.SKIPPED_NOT_ELIGIBLE + assert fake.create_ref_calls == [] + + +async def test_regression_detected_never_opens_a_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.REGRESSION_DETECTED) + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.SKIPPED_NOT_ELIGIBLE + + +async def test_partially_verified_without_policy_never_opens_a_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.PARTIALLY_VERIFIED) + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.SKIPPED_NOT_ELIGIBLE + + +async def test_partially_verified_with_operator_policy_opens_a_marked_pr(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.PARTIALLY_VERIFIED, policy=MigrationPRPolicy(allow_partially_verified=True)) + assert plan.eligibility is MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.OPENED + + +async def test_human_required_never_touches_github(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.HUMAN_REQUIRED) + assert plan.eligibility is MigrationPREligibility.PLAN_ONLY + result = await _publisher(session_factory, fake).publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert result.status is MigrationPRStatus.PLANNED + assert fake.create_ref_calls == [] and fake.create_pull_request_calls == [] + + +async def test_closed_pr_is_never_reopened_automatically(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + publisher = _publisher(session_factory, fake) + first = await publisher.publish(plan=plan, patch=_patch(), mode=MigrationPRPublicationMode.PUBLISH) + assert first.number is not None + fake.close_pull_request(first.number) + + second = await publisher.publish(plan=plan, patch=_patch(content="v2"), mode=MigrationPRPublicationMode.PUBLISH) + assert second.status is MigrationPRStatus.NO_OP_UNCHANGED + assert second.number == first.number + assert len(fake.create_pull_request_calls) == 1 # never reopened, never duplicated + + +async def test_check_run_is_reconciled_not_duplicated(session_factory: async_sessionmaker[AsyncSession]) -> None: + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + publisher = _publisher(session_factory, fake) + # A GitHub check run lives on one exact commit SHA -- republishing + # identical patch content produces the identical tree/commit (never a + # new SHA), so this is the one case reconciliation actually applies: + # same external_id *and* same head_sha both times. + await publisher.publish(plan=plan, patch=_patch(content="same"), mode=MigrationPRPublicationMode.PUBLISH) + await publisher.publish(plan=plan, patch=_patch(content="same"), mode=MigrationPRPublicationMode.PUBLISH) + assert len(fake.checks) == 1 + + +async def test_regenerated_patch_gets_its_own_check_run_on_its_own_commit( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + """A force-moved branch's new commit is a different SHA -- GitHub + check runs are inherently per-commit, so a *changed* patch legitimately + produces a second check run (on the new tip), never an update to a + check attached to a commit that is no longer the branch head.""" + + fake = FakeMigrationGitHubPublisher() + fake.set_ref(owner=OWNER, repository=REPO, ref="heads/main", sha=BASE_SHA) + plan = _plan(outcome=VerificationOutcome.VERIFIED) + publisher = _publisher(session_factory, fake) + await publisher.publish(plan=plan, patch=_patch(content="v1"), mode=MigrationPRPublicationMode.PUBLISH) + await publisher.publish(plan=plan, patch=_patch(content="v2"), mode=MigrationPRPublicationMode.PUBLISH) + assert len(fake.checks) == 2 + assert len({c.head_sha for c in fake.checks}) == 2 diff --git a/tests/integration/test_migration_pr_real_github_e2e.py b/tests/integration/test_migration_pr_real_github_e2e.py new file mode 100644 index 0000000..b322310 --- /dev/null +++ b/tests/integration/test_migration_pr_real_github_e2e.py @@ -0,0 +1,60 @@ +"""M9.11: optional, opt-in harness for a *real* GitHub App migration-PR +publish -- never run as part of the normal test suite. + +This test is skipped unless **all** of the following are explicitly set, +which they never are in CI or in a normal developer run: + +- ``PATCHFROG_MIGRATION_PR_REAL_GITHUB_E2E=1`` -- the actual opt-in; every + other variable below is inert without this one. +- ``PATCHFROG_MIGRATION_PR_E2E_INSTALLATION_ID`` -- a GitHub App + installation ID the operator controls. +- ``PATCHFROG_MIGRATION_PR_E2E_REPOSITORY`` -- ``owner/repo`` of a + disposable fixture repository that installation may write to. Never a + production repository, and never PatchFrog's own. + +No secret value is read or printed by this module itself -- +``GitHubClient``/``InstallationTokenProvider`` resolve credentials the +same way the production worker does (`patchfrog/config/settings.py`), +entirely out of this test's own code path. This module exists so the +harness exists (M9.11's own requirement to "add" one) without ever +exercising it here: doing so would mutate a real GitHub repository, which +this milestone's own hard constraints forbid without already-configured +credentials and explicit, separate operator approval. + +A real run (when someone deliberately opts in, outside of this milestone) +would: build a trivially `VERIFIED` `MigrationPRPlan` against the fixture +repository's current default-branch head, publish it once via +`MigrationPRPublisher` in `PUBLISH` mode against the real +`GitHubClientMigrationPublisher`, assert a real PR now exists, then close +it -- and never merge it. +""" + +from __future__ import annotations + +import os + +import pytest + +_ENABLED = os.environ.get("PATCHFROG_MIGRATION_PR_REAL_GITHUB_E2E") == "1" + +pytestmark = pytest.mark.skipif( + not _ENABLED, + reason=( + "opt-in only -- set PATCHFROG_MIGRATION_PR_REAL_GITHUB_E2E=1 (plus " + "PATCHFROG_MIGRATION_PR_E2E_INSTALLATION_ID and " + "PATCHFROG_MIGRATION_PR_E2E_REPOSITORY) to run this against a real, " + "disposable fixture repository; never enabled in CI" + ), +) + + +def test_real_publish_opens_and_reconciles_one_pr() -> None: + """Deliberately not implemented in this milestone -- see the module + docstring. Reaching this line at all means the opt-in gate above has + failed open, which would itself be the bug worth catching; the body + fails loudly rather than silently doing nothing under a green check.""" + + pytest.fail( + "the real-GitHub migration PR E2E harness is not implemented in this milestone -- " + "see docs/migration-pr.md's 'Known limitations' section" + ) diff --git a/tests/integration/test_migration_publish_cli.py b/tests/integration/test_migration_publish_cli.py new file mode 100644 index 0000000..172f1b3 --- /dev/null +++ b/tests/integration/test_migration_publish_cli.py @@ -0,0 +1,65 @@ +"""M9.10: ``patchfrog migrations publish --dry-run`` -- text and JSON +forms, and the CLI's own safety gates (real publication and multi-repo +fan-out are both rejected, never silently narrowed).""" + +from __future__ import annotations + +import json + +import pytest + +from patchfrog.cli import main +from tests.support.upstream_cases import CASES_ROOT + +DEMO_VERIFIED = CASES_ROOT / "demo_verified" + + +def _argv(*, json_output: bool = False, dry_run: bool = True, repository: str | None = "acme-org/widgets") -> list[str]: + argv = [ + "migrations", "publish", "--old", str(DEMO_VERIFIED / "old.yaml"), "--new", str(DEMO_VERIFIED / "new.yaml"), + "--hints", str(DEMO_VERIFIED / "hints.yaml"), "--repo", f"demo={DEMO_VERIFIED / 'repo'}", + ] + if repository is not None: + argv += ["--repository", repository] + if dry_run: + argv.append("--dry-run") + if json_output: + argv.append("--json") + return argv + + +def test_publish_dry_run_never_touches_github_and_prints_a_plan(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv()) == 0 + text = capsys.readouterr().out + assert "Eligibility:" in text + assert "Proposed branch: patchfrog/migrate/acme-ai/" in text + assert "PR title:" in text + assert "PR body:" in text + + +def test_publish_dry_run_json(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv(json_output=True)) == 0 + payload = json.loads(capsys.readouterr().out) + assert payload["eligibility"] in {"auto_open", "open_with_operator_approval", "plan_only", "not_eligible"} + assert payload["linkage"]["repository"] == "acme-org/widgets" + assert payload["branch_name"].startswith("patchfrog/migrate/acme-ai/") + assert "identity_key" in payload["linkage"] + + +def test_publish_without_dry_run_is_rejected(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv(dry_run=False)) == 2 + err = capsys.readouterr().err + assert "only --dry-run is supported" in err + + +def test_publish_without_repository_is_rejected(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv(repository=None)) == 2 + err = capsys.readouterr().err + assert "--repository" in err + + +def test_publish_rejects_more_than_one_repo(capsys: pytest.CaptureFixture[str]) -> None: + argv = [*_argv(), "--repo", f"second={DEMO_VERIFIED / 'repo'}"] + assert main(argv) == 2 + err = capsys.readouterr().err + assert "exactly one --repo" in err diff --git a/tests/integration/test_migration_verification_service.py b/tests/integration/test_migration_verification_service.py new file mode 100644 index 0000000..e28ec23 --- /dev/null +++ b/tests/integration/test_migration_verification_service.py @@ -0,0 +1,110 @@ +"""M8 end-to-end: real, sandboxed executable evidence against the two +bundled demo repositories. + +``demo_verified`` (a single, fully-automatic consumer) demonstrates the +full VERIFIED path with real baseline-vs-patched evidence. ``demo`` (the +M7 bundled demo, which has one human-required step -- the removed +``stream`` parameter) demonstrates the HUMAN_REQUIRED path. Both reuse +:class:`~patchfrog.executable_verification.sandbox.VerificationSandbox` +directly; this test is skipped where the sandbox itself is unavailable +(see :func:`patchfrog.executable_verification.sandbox.is_sandbox_available`), +exactly like Executable Verification's own tests do. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from patchfrog.dependencies.discovery import discover_dependencies +from patchfrog.executable_verification.sandbox import is_sandbox_available +from patchfrog.migration.domain import ( + AutoFixEligibility, + GeneratedPatch, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + ResidualRisk, +) +from patchfrog.migration.generator import generate_patch +from patchfrog.migration.planner import plan_migration +from patchfrog.migration_verification.domain import ( + BaselineComparisonOutcome, + MigrationEvidenceBundle, + VerificationOutcome, +) +from patchfrog.migration_verification.service import run_migration_verification +from patchfrog.upstream.blast_radius import BlastRadius +from patchfrog.upstream.events import build_contract_change, load_contract_file +from patchfrog.upstream.hints import load_hints +from patchfrog.upstream.workspace import adapters_for_target, analyze_inventory + +DEMO_CASES_ROOT = Path(__file__).resolve().parent.parent / "fixtures" / "upstream_changes" + +pytestmark = pytest.mark.skipif(not is_sandbox_available(), reason="verification sandbox unavailable on this host") + + +def _run(name: str) -> tuple[MigrationPlan, GeneratedPatch, MigrationEvidenceBundle]: + root_dir = DEMO_CASES_ROOT / name + hints = load_hints((root_dir / "hints.yaml").read_text()) + event = build_contract_change(load_contract_file(root_dir / "old.yaml"), load_contract_file(root_dir / "new.yaml"), + hints=hints) + repo_root = root_dir / "repo" + inventory = discover_dependencies(repo_root, repository=name, adapters=adapters_for_target(event.target)) + impact = analyze_inventory(inventory, event, hints=hints, root=repo_root) + plan = plan_migration(event, impact, inventory, hints=hints) + patch = generate_patch(plan, repo_root) + bundle = asyncio.run( + run_migration_verification( + plan=plan, patch=patch, blast_radii=impact.blast_radii, root=repo_root, commit_sha="deadbeef", + extra_workspace_paths=(root_dir / "sdk_stub",), + ) + ) + return plan, patch, bundle + + +def test_fully_automatic_migration_reaches_verified_with_strong_evidence() -> None: + plan, _patch, bundle = _run("demo_verified") + assert plan.status is MigrationStatus.PLANNED + assert not plan.unresolved_steps + assert bundle.outcome is VerificationOutcome.VERIFIED + assert bundle.evidence_strength.value == "strong" + assert bundle.residual_risk.value == "low" + assert not bundle.coverage.unresolved_mandatory + assert any(c.outcome is BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_PASS for c in bundle.baseline_comparisons) + + +def test_migration_with_a_human_required_step_reaches_human_required() -> None: + plan, _patch, bundle = _run("demo") + assert plan.unresolved_steps # the removed `stream` argument at workers/summary.py + assert bundle.outcome is VerificationOutcome.HUMAN_REQUIRED + # Everything automatable still checked out cleanly. + assert not bundle.coverage.failed_mandatory + + +def test_evidence_bundle_is_bound_to_exact_patch_and_change_identity() -> None: + _plan, patch, bundle = _run("demo_verified") + assert bundle.patch_fingerprint == patch.fingerprint + assert bundle.repository_head_sha == "deadbeef" + other_fp = bundle.bundle_fingerprint + assert other_fp and len(other_fp) == 64 + + +async def _no_patch_bundle() -> MigrationEvidenceBundle: + target = MigrationTarget("repo", "app/x.py", None, None, "x", "sdk_call", "python") + step = MigrationStep("s1", target, MigrationStrategy.MANUAL_REVIEW, AutoFixEligibility.HUMAN_REQUIRED, (), (), + "", "", "", (), "") + plan = MigrationPlan("fp", "repo", None, (), (step,), ResidualRisk.HIGH, MigrationStatus.HUMAN_REQUIRED) + blast_radii: tuple[BlastRadius, ...] = () + return await run_migration_verification( + plan=plan, patch=None, blast_radii=blast_radii, root=Path("."), commit_sha="", + ) + + +def test_no_patch_at_all_is_human_required_never_crashes() -> None: + bundle = asyncio.run(_no_patch_bundle()) + assert bundle.outcome is VerificationOutcome.HUMAN_REQUIRED diff --git a/tests/integration/test_migration_verification_store.py b/tests/integration/test_migration_verification_store.py new file mode 100644 index 0000000..c211e1c --- /dev/null +++ b/tests/integration/test_migration_verification_store.py @@ -0,0 +1,187 @@ +"""M8.9 persistence: idempotent evidence-bundle storage, DB-backed.""" + +from __future__ import annotations + +import uuid + +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from patchfrog.dependencies.discovery import discover_dependencies +from patchfrog.migration.domain import ( + AutoFixEligibility, + EditOperation, + GeneratedPatch, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + PatchOrigin, + ResidualRisk, + SafetyGateResult, + StepOutcome, + StepResult, +) +from patchfrog.migration.generator import generate_patch +from patchfrog.migration.planner import plan_migration +from patchfrog.migration.store import MigrationStore, build_linkage +from patchfrog.migration_verification.decision import decide_outcome +from patchfrog.migration_verification.domain import ( + EvidenceStrength, + MigrationEvidenceBundle, + MigrationVerificationPlan, +) +from patchfrog.migration_verification.evidence import ( + build_coverage, + build_evidence_bundle, + compute_residual_risk, +) +from patchfrog.migration_verification.store import MigrationVerificationStore +from patchfrog.persistence.models.migration_verification import MigrationVerificationRunModel +from patchfrog.persistence.repositories import RepositoryRepository +from patchfrog.upstream.store import UpstreamChangeStore +from patchfrog.upstream.workspace import adapters_for_target, analyze_inventory +from tests.support.upstream_cases import CASES_ROOT, load_case + + +async def _repository(session_factory: async_sessionmaker[AsyncSession]) -> uuid.UUID: + name = f"m8-{uuid.uuid4().hex[:8]}" + async with session_factory() as session: + row = await RepositoryRepository().upsert( + session, github_repository_id=uuid.uuid4().int % (2**62), owner="org", name=name, + full_name=f"org/{name}", installation_id=0, + ) + await session.commit() + return row.id + + +def _target() -> MigrationTarget: + return MigrationTarget("repo", "app/chat.py", "generate_reply", 3, "chat.create", "sdk_call", "python") + + +def _bundle( + *, change_fingerprint: str, patch_fingerprint: str, outcome_strength: EvidenceStrength +) -> MigrationEvidenceBundle: + plan = MigrationVerificationPlan( + change_fingerprint=change_fingerprint, patch_fingerprint=patch_fingerprint, requirements=(), steps=(), + test_selections=(), + ) + coverage = build_coverage(plan, step_evidence=(), contract_checks=()) + outcome, reasons = decide_outcome( + plan, coverage=coverage, evidence_strength=outcome_strength, regressions=(), has_unresolved_human_steps=False, + ) + residual_risk = compute_residual_risk(coverage, evidence_strength=outcome_strength, has_unresolved_human_steps=False) + return build_evidence_bundle( + plan=plan, repository="org/repo", repository_head_sha="deadbeef", step_evidence=(), contract_checks=(), + baseline_comparisons=(), coverage=coverage, evidence_strength=outcome_strength, residual_risk=residual_risk, + outcome=outcome, outcome_reasons=reasons, + ) + + +async def test_verification_run_persistence_is_idempotent(session_factory: async_sessionmaker[AsyncSession]) -> None: + case = load_case(CASES_ROOT / "demo") + root = case.repositories["demo"] + inventory = discover_dependencies(root, repository="demo", adapters=adapters_for_target(case.event.target)) + impact = analyze_inventory(inventory, case.event, hints=case.hints, root=root) + plan = plan_migration(case.event, impact, inventory, hints=case.hints) + patch = generate_patch(plan, root) + + upstream_store = UpstreamChangeStore() + migration_store = MigrationStore() + verification_store = MigrationVerificationStore() + repository_id = await _repository(session_factory) + + async with session_factory() as session: + event_row, _ = await upstream_store.record_event(session, case.event) + plan_row, base_fingerprint, _ = await migration_store.record_plan( + session, event_id=event_row.id, repository_id=repository_id, plan=plan, root=root + ) + linkage = build_linkage(plan, patch, base_content_fingerprint=base_fingerprint, + plan_fingerprint=plan_row.plan_fingerprint) + patch_row, _ = await migration_store.record_patch(session, plan_id=plan_row.id, patch=patch, linkage=linkage) + await session.commit() + patch_id = patch_row.id + + bundle = _bundle( + change_fingerprint=plan.change_fingerprint, patch_fingerprint=patch.fingerprint, + outcome_strength=EvidenceStrength.NONE, + ) + + async with session_factory() as session: + row, created = await verification_store.record_run(session, patch_id=patch_id, bundle=bundle) + await session.commit() + assert created + assert row.outcome == bundle.outcome.value + + # Re-recording the identical (patch, verification-plan-fingerprint) + # pair is a no-op write -- idempotent, never a duplicate row. + async with session_factory() as session: + row2, created2 = await verification_store.record_run(session, patch_id=patch_id, bundle=bundle) + await session.commit() + assert not created2 and row2.id == row.id + async with session_factory() as session: + count = ( + await session.execute( + select(func.count()).select_from(MigrationVerificationRunModel).where( + MigrationVerificationRunModel.patch_id == patch_id + ) + ) + ).scalar_one() + assert count == 1 + + async with session_factory() as session: + latest = await verification_store.latest_for_patch(session, patch_id=patch_id) + assert latest is not None and latest.id == row.id + + +async def test_a_different_patch_fingerprint_never_reuses_old_evidence( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + """M9.8's integrity requirement starts at the storage layer: a + verification run is keyed by (patch_id, verification-plan + fingerprint) -- a different generated patch is a different + ``patch_id`` by construction (migration_patches' own unique + constraint), so its evidence can never collide with another patch's.""" + + target = _target() + step = MigrationStep("s1", target, MigrationStrategy.RENAME_SYMBOL, AutoFixEligibility.AUTO_SAFE, (), (), "o", + "n", "n", (), "", operation=EditOperation.of("op")) + plan = MigrationPlan("fp", "org/repo", "sha", (), (step,), ResidualRisk.LOW, MigrationStatus.PLANNED) + patch_a = GeneratedPatch(PatchOrigin.DETERMINISTIC, "diff-a", ("app/chat.py",), {"app/chat.py": "a"}, + (StepResult("s1", StepOutcome.APPLIED, "d"),), (SafetyGateResult("g", True, "ok"),)) + patch_b = GeneratedPatch(PatchOrigin.DETERMINISTIC, "diff-b", ("app/chat.py",), {"app/chat.py": "b"}, + (StepResult("s1", StepOutcome.APPLIED, "d"),), (SafetyGateResult("g", True, "ok"),)) + assert patch_a.fingerprint != patch_b.fingerprint + + upstream_store = UpstreamChangeStore() + migration_store = MigrationStore() + verification_store = MigrationVerificationStore() + repository_id = await _repository(session_factory) + case = load_case(CASES_ROOT / "demo") + + async with session_factory() as session: + event_row, _ = await upstream_store.record_event(session, case.event) + plan_row, base_fingerprint, _ = await migration_store.record_plan( + session, event_id=event_row.id, repository_id=repository_id, plan=plan, root=case.repositories["demo"], + ) + linkage_a = build_linkage(plan, patch_a, base_content_fingerprint=base_fingerprint, + plan_fingerprint=plan_row.plan_fingerprint) + linkage_b = build_linkage(plan, patch_b, base_content_fingerprint=base_fingerprint, + plan_fingerprint=plan_row.plan_fingerprint) + patch_row_a, _ = await migration_store.record_patch(session, plan_id=plan_row.id, patch=patch_a, linkage=linkage_a) + patch_row_b, _ = await migration_store.record_patch(session, plan_id=plan_row.id, patch=patch_b, linkage=linkage_b) + await session.commit() + + bundle_a = _bundle(change_fingerprint=plan.change_fingerprint, patch_fingerprint=patch_a.fingerprint, + outcome_strength=EvidenceStrength.STRONG) + bundle_b = _bundle(change_fingerprint=plan.change_fingerprint, patch_fingerprint=patch_b.fingerprint, + outcome_strength=EvidenceStrength.NONE) + + async with session_factory() as session: + row_a, _ = await verification_store.record_run(session, patch_id=patch_row_a.id, bundle=bundle_a) + row_b, _ = await verification_store.record_run(session, patch_id=patch_row_b.id, bundle=bundle_b) + await session.commit() + + assert row_a.id != row_b.id + assert row_a.patch_fingerprint != row_b.patch_fingerprint diff --git a/tests/integration/test_migration_verify_cli.py b/tests/integration/test_migration_verify_cli.py new file mode 100644 index 0000000..33b88e2 --- /dev/null +++ b/tests/integration/test_migration_verify_cli.py @@ -0,0 +1,103 @@ +"""M8.13: ``patchfrog migrations verify`` -- text and JSON forms, and +``--persist``.""" + +from __future__ import annotations + +import asyncio +import json +import uuid + +import pytest +from sqlalchemy import select +from sqlalchemy import text as sa_text +from sqlalchemy.ext.asyncio import async_sessionmaker + +from patchfrog.cli import main +from patchfrog.config.settings import get_settings +from patchfrog.executable_verification.sandbox import is_sandbox_available +from patchfrog.persistence.models.repository import RepositoryModel +from tests.support.postgres import POSTGRES_TEST_URL, postgres_engine_or_skip +from tests.support.upstream_cases import CASES_ROOT + +DEMO_VERIFIED = CASES_ROOT / "demo_verified" + +pytestmark = pytest.mark.skipif(not is_sandbox_available(), reason="verification sandbox unavailable on this host") + + +def _argv(*, json_output: bool = False) -> list[str]: + argv = [ + "migrations", "verify", "--old", str(DEMO_VERIFIED / "old.yaml"), "--new", str(DEMO_VERIFIED / "new.yaml"), + "--hints", str(DEMO_VERIFIED / "hints.yaml"), "--repo", f"demo={DEMO_VERIFIED / 'repo'}", + ] + if json_output: + argv.append("--json") + return argv + + +def test_migrations_verify_text(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv()) == 0 + text = capsys.readouterr().out + assert "Migration verification for demo:" in text + assert "Verification outcome:" in text + assert "Residual risk:" in text + + +def test_migrations_verify_json(capsys: pytest.CaptureFixture[str]) -> None: + assert main(_argv(json_output=True)) == 0 + payload = json.loads(capsys.readouterr().out) + verification = payload["verification"]["demo"] + assert verification["outcome"] in {"verified", "partially_verified", "unverified", "human_required"} + assert "bundle_fingerprint" in verification + + +async def _cleanup(full_names: list[str], fingerprints: list[str]) -> None: + engine = await postgres_engine_or_skip("migration_plans") + try: + session_factory = async_sessionmaker(engine, expire_on_commit=False) + async with session_factory() as session: + for name in full_names: + row = ( + await session.execute(select(RepositoryModel).where(RepositoryModel.full_name == name)) + ).scalar_one_or_none() + if row is not None: + await session.delete(row) + await session.commit() + for fingerprint in fingerprints: + await session.execute( + sa_text("DELETE FROM external_change_events WHERE fingerprint = :f"), {"f": fingerprint} + ) + await session.commit() + finally: + await engine.dispose() + + +def test_migrations_verify_persist_records_verification_run( + capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + asyncio.run(_cleanup([], [])) # ensure Postgres is reachable, or skip + monkeypatch.setenv("DATABASE_URL", POSTGRES_TEST_URL) + get_settings.cache_clear() + name = f"test/m8-cli-{uuid.uuid4().hex[:8]}" + argv = [ + "migrations", "verify", "--old", str(DEMO_VERIFIED / "old.yaml"), "--new", str(DEMO_VERIFIED / "new.yaml"), + "--hints", str(DEMO_VERIFIED / "hints.yaml"), "--provider", f"acme-{uuid.uuid4().hex[:6]}", + "--repo", f"{name}={DEMO_VERIFIED / 'repo'}", "--persist", "--json", + ] + fingerprint = "" + try: + assert main(argv) == 0 + payload = json.loads(capsys.readouterr().out) + fingerprint = payload["event"]["fingerprint"] + persisted = payload["persisted"][name] + assert persisted["patch_created"] is True + assert persisted["verification_run_created"] is True + + # Re-running is idempotent: the same verification run is reused. + assert main(argv) == 0 + payload2 = json.loads(capsys.readouterr().out) + persisted2 = payload2["persisted"][name] + assert persisted2["verification_run_id"] == persisted["verification_run_id"] + assert persisted2["verification_run_created"] is False + finally: + asyncio.run(_cleanup([name], [fingerprint] if fingerprint else [])) + get_settings.cache_clear() diff --git a/tests/unit/test_github_client_git.py b/tests/unit/test_github_client_git.py new file mode 100644 index 0000000..0a50a7b --- /dev/null +++ b/tests/unit/test_github_client_git.py @@ -0,0 +1,182 @@ +"""M9.2/M9.6: the new Git Data API + pull-request-creation surface on +:class:`GitHubClient` -- branch/commit/tree creation and PR open/list/ +update. Every call is intercepted by ``respx``; no real GitHub request is +ever made.""" + +from __future__ import annotations + +import json +from collections.abc import AsyncIterator + +import httpx +import pytest +import respx + +from patchfrog.domain.github_git import GitTreeEntry +from patchfrog.github.client import GitHubClient +from patchfrog.github.errors import GitHubResponseError, GitHubUnprocessableError + +API_BASE = "https://api.github.com" + + +class _TokenProvider: + async def get_token(self, installation_id: int) -> str: + return "token" + + +@pytest.fixture +async def http_client() -> AsyncIterator[httpx.AsyncClient]: + async with httpx.AsyncClient() as client: + yield client + + +def _client(http_client: httpx.AsyncClient) -> GitHubClient: + return GitHubClient(http_client=http_client, token_provider=_TokenProvider(), api_base_url=API_BASE) # type: ignore[arg-type] + + +@respx.mock +async def test_get_ref_returns_sha(http_client: httpx.AsyncClient) -> None: + respx.get(f"{API_BASE}/repos/octo/repo/git/ref/heads/main").mock( + return_value=httpx.Response(200, json={"object": {"sha": "a" * 40}}) + ) + sha = await _client(http_client).get_ref(installation_id=1, owner="octo", repository="repo", ref="heads/main") + assert sha == "a" * 40 + + +@respx.mock +async def test_get_ref_returns_none_when_missing(http_client: httpx.AsyncClient) -> None: + respx.get(f"{API_BASE}/repos/octo/repo/git/ref/heads/gone").mock(return_value=httpx.Response(404, json={})) + sha = await _client(http_client).get_ref(installation_id=1, owner="octo", repository="repo", ref="heads/gone") + assert sha is None + + +@respx.mock +async def test_create_ref_posts_full_ref_and_sha(http_client: httpx.AsyncClient) -> None: + route = respx.post(f"{API_BASE}/repos/octo/repo/git/refs").mock( + return_value=httpx.Response(201, json={"ref": "refs/heads/x", "object": {"sha": "b" * 40}}) + ) + await _client(http_client).create_ref( + installation_id=1, owner="octo", repository="repo", ref="refs/heads/patchfrog/migrate/acme/abc123", sha="b" * 40, + ) + payload = json.loads(route.calls.last.request.content) + assert payload == {"ref": "refs/heads/patchfrog/migrate/acme/abc123", "sha": "b" * 40} + + +@respx.mock +async def test_create_ref_collision_raises_unprocessable(http_client: httpx.AsyncClient) -> None: + respx.post(f"{API_BASE}/repos/octo/repo/git/refs").mock( + return_value=httpx.Response(422, json={"message": "Reference already exists"}) + ) + with pytest.raises(GitHubUnprocessableError): + await _client(http_client).create_ref( + installation_id=1, owner="octo", repository="repo", ref="refs/heads/x", sha="c" * 40, + ) + + +@respx.mock +async def test_update_ref_patches_sha_and_force(http_client: httpx.AsyncClient) -> None: + route = respx.patch(f"{API_BASE}/repos/octo/repo/git/refs/heads/x").mock( + return_value=httpx.Response(200, json={"ref": "refs/heads/x", "object": {"sha": "d" * 40}}) + ) + await _client(http_client).update_ref( + installation_id=1, owner="octo", repository="repo", ref="heads/x", sha="d" * 40, force=True, + ) + payload = json.loads(route.calls.last.request.content) + assert payload == {"sha": "d" * 40, "force": True} + + +@respx.mock +async def test_get_commit_tree_sha(http_client: httpx.AsyncClient) -> None: + respx.get(f"{API_BASE}/repos/octo/repo/git/commits/{'e' * 40}").mock( + return_value=httpx.Response(200, json={"tree": {"sha": "f" * 40}}) + ) + tree_sha = await _client(http_client).get_commit_tree_sha( + installation_id=1, owner="octo", repository="repo", commit_sha="e" * 40, + ) + assert tree_sha == "f" * 40 + + +@respx.mock +async def test_create_tree_sends_file_content_entries(http_client: httpx.AsyncClient) -> None: + route = respx.post(f"{API_BASE}/repos/octo/repo/git/trees").mock( + return_value=httpx.Response(201, json={"sha": "g" * 40}) + ) + tree_sha = await _client(http_client).create_tree( + installation_id=1, owner="octo", repository="repo", base_tree_sha="h" * 40, + entries=[GitTreeEntry(path="app/chat.py", content="new content")], + ) + assert tree_sha == "g" * 40 + payload = json.loads(route.calls.last.request.content) + assert payload["base_tree"] == "h" * 40 + assert payload["tree"] == [{"path": "app/chat.py", "mode": "100644", "type": "blob", "content": "new content"}] + + +@respx.mock +async def test_create_commit_sends_message_tree_and_parent(http_client: httpx.AsyncClient) -> None: + route = respx.post(f"{API_BASE}/repos/octo/repo/git/commits").mock( + return_value=httpx.Response(201, json={"sha": "i" * 40}) + ) + commit_sha = await _client(http_client).create_commit( + installation_id=1, owner="octo", repository="repo", message="migrate acme-ai", tree_sha="j" * 40, + parent_sha="k" * 40, + ) + assert commit_sha == "i" * 40 + payload = json.loads(route.calls.last.request.content) + assert payload == {"message": "migrate acme-ai", "tree": "j" * 40, "parents": ["k" * 40]} + + +@respx.mock +async def test_create_pull_request_posts_title_body_head_base(http_client: httpx.AsyncClient) -> None: + response = { + "number": 42, "title": "Migrate acme-ai", "body": "dossier", "user": {"login": "patchfrog[bot]"}, + "base": {"ref": "main", "sha": "l" * 40}, "head": {"ref": "patchfrog/migrate/acme/abc", "sha": "m" * 40}, + "html_url": "https://github.com/octo/repo/pull/42", "state": "open", "merged": False, + } + route = respx.post(f"{API_BASE}/repos/octo/repo/pulls").mock(return_value=httpx.Response(201, json=response)) + pr = await _client(http_client).create_pull_request( + installation_id=1, owner="octo", repository="repo", title="Migrate acme-ai", body="dossier", + head="patchfrog/migrate/acme/abc", base="main", + ) + assert pr.number == 42 and pr.html_url == "https://github.com/octo/repo/pull/42" + payload = json.loads(route.calls.last.request.content) + assert payload == {"title": "Migrate acme-ai", "body": "dossier", "head": "patchfrog/migrate/acme/abc", "base": "main"} + + +@respx.mock +async def test_list_pull_requests_filters_by_head(http_client: httpx.AsyncClient) -> None: + response = [ + { + "number": 7, "title": "t", "body": None, "user": {"login": "u"}, "base": {"ref": "main", "sha": "a" * 40}, + "head": {"ref": "patchfrog/migrate/acme/abc", "sha": "b" * 40}, + "html_url": "https://github.com/octo/repo/pull/7", "state": "open", "merged": False, + } + ] + route = respx.get(f"{API_BASE}/repos/octo/repo/pulls").mock(return_value=httpx.Response(200, json=response)) + pull_requests = await _client(http_client).list_pull_requests( + installation_id=1, owner="octo", repository="repo", head="octo:patchfrog/migrate/acme/abc", + ) + assert [pr.number for pr in pull_requests] == [7] + assert route.calls.last.request.url.params["head"] == "octo:patchfrog/migrate/acme/abc" + + +@respx.mock +async def test_update_pull_request_patches_only_given_fields(http_client: httpx.AsyncClient) -> None: + response = { + "number": 7, "title": "new title", "body": "old body", "user": {"login": "u"}, + "base": {"ref": "main", "sha": "a" * 40}, "head": {"ref": "x", "sha": "b" * 40}, + "html_url": "https://github.com/octo/repo/pull/7", "state": "open", "merged": False, + } + route = respx.patch(f"{API_BASE}/repos/octo/repo/pulls/7").mock(return_value=httpx.Response(200, json=response)) + pr = await _client(http_client).update_pull_request( + installation_id=1, owner="octo", repository="repo", number=7, title="new title", + ) + assert pr.title == "new title" + payload = json.loads(route.calls.last.request.content) + assert payload == {"title": "new title"} + + +@respx.mock +async def test_malformed_ref_response_raises(http_client: httpx.AsyncClient) -> None: + respx.get(f"{API_BASE}/repos/octo/repo/git/ref/heads/main").mock(return_value=httpx.Response(200, json={})) + with pytest.raises(GitHubResponseError): + await _client(http_client).get_ref(installation_id=1, owner="octo", repository="repo", ref="heads/main") diff --git a/tests/unit/test_migration_pr_branch_eligibility.py b/tests/unit/test_migration_pr_branch_eligibility.py new file mode 100644 index 0000000..5e8d285 --- /dev/null +++ b/tests/unit/test_migration_pr_branch_eligibility.py @@ -0,0 +1,69 @@ +"""M9.2 / M9.3: deterministic branch naming and publication eligibility.""" + +from __future__ import annotations + +from patchfrog.migration_pr.branch import migration_branch_name +from patchfrog.migration_pr.domain import MigrationPREligibility, MigrationPRPolicy +from patchfrog.migration_pr.eligibility import determine_eligibility +from patchfrog.migration_verification.domain import VerificationOutcome + + +def test_branch_name_is_stable_for_same_identity() -> None: + a = migration_branch_name(provider="acme-ai", change_fingerprint="abc123def456" + "0" * 52) + b = migration_branch_name(provider="acme-ai", change_fingerprint="abc123def456" + "0" * 52) + assert a == b + assert a == "patchfrog/migrate/acme-ai/abc123def456" + + +def test_branch_name_differs_for_different_change_fingerprint() -> None: + a = migration_branch_name(provider="acme-ai", change_fingerprint="a" * 64) + b = migration_branch_name(provider="acme-ai", change_fingerprint="b" * 64) + assert a != b + + +def test_branch_name_sanitizes_unsafe_provider_chars() -> None: + name = migration_branch_name(provider="Acme AI/Beta!", change_fingerprint="c" * 64) + assert name.startswith("patchfrog/migrate/acme-ai-beta") + assert "/" not in name.split("patchfrog/migrate/", 1)[1].split("/", 1)[0] + + +def test_branch_name_falls_back_when_no_provider() -> None: + name = migration_branch_name(provider=None, change_fingerprint="d" * 64) + assert name == "patchfrog/migrate/dependency/" + "d" * 12 + + +def test_verified_is_auto_open() -> None: + eligibility, _ = determine_eligibility(VerificationOutcome.VERIFIED, policy=MigrationPRPolicy()) + assert eligibility is MigrationPREligibility.AUTO_OPEN + + +def test_partially_verified_is_not_eligible_by_default() -> None: + eligibility, reason = determine_eligibility(VerificationOutcome.PARTIALLY_VERIFIED, policy=MigrationPRPolicy()) + assert eligibility is MigrationPREligibility.NOT_ELIGIBLE + assert "allow_partially_verified" in reason + + +def test_partially_verified_is_open_with_approval_when_policy_allows() -> None: + eligibility, reason = determine_eligibility( + VerificationOutcome.PARTIALLY_VERIFIED, policy=MigrationPRPolicy(allow_partially_verified=True), + ) + assert eligibility is MigrationPREligibility.OPEN_WITH_OPERATOR_APPROVAL + assert "visibly marked" in reason + + +def test_human_required_is_plan_only() -> None: + eligibility, _ = determine_eligibility(VerificationOutcome.HUMAN_REQUIRED, policy=MigrationPRPolicy()) + assert eligibility is MigrationPREligibility.PLAN_ONLY + + +def test_unverified_failed_and_regression_are_never_eligible() -> None: + for outcome in (VerificationOutcome.UNVERIFIED, VerificationOutcome.FAILED, VerificationOutcome.REGRESSION_DETECTED): + eligibility, _ = determine_eligibility(outcome, policy=MigrationPRPolicy()) + assert eligibility is MigrationPREligibility.NOT_ELIGIBLE + + +def test_policy_never_relaxes_regression_or_failed() -> None: + policy = MigrationPRPolicy(allow_partially_verified=True) + for outcome in (VerificationOutcome.FAILED, VerificationOutcome.REGRESSION_DETECTED, VerificationOutcome.UNVERIFIED): + eligibility, _ = determine_eligibility(outcome, policy=policy) + assert eligibility is MigrationPREligibility.NOT_ELIGIBLE diff --git a/tests/unit/test_migration_pr_integrity.py b/tests/unit/test_migration_pr_integrity.py new file mode 100644 index 0000000..2ba90d0 --- /dev/null +++ b/tests/unit/test_migration_pr_integrity.py @@ -0,0 +1,110 @@ +"""M9.8: evidence/patch integrity check -- a `MigrationEvidenceBundle` +computed for one upstream change / patch / repository state must never be +silently treated as evidence for a different one.""" + +from __future__ import annotations + +import pytest + +from patchfrog.migration.domain import ( + GeneratedPatch, + PatchOrigin, + SafetyGateResult, + StepOutcome, + StepResult, +) +from patchfrog.migration_pr.integrity import MigrationPRIntegrityError, verify_evidence_integrity +from patchfrog.migration_verification.domain import ( + EvidenceStrength, + MigrationEvidenceBundle, + MigrationVerificationPlan, + ResidualRisk, + VerificationCoverage, + VerificationOutcome, +) +from patchfrog.migration_verification.evidence import build_evidence_bundle +from patchfrog.upstream.domain import ExternalChangeEvent +from patchfrog.upstream.events import build_contract_change, load_contract_file +from patchfrog.upstream.hints import EMPTY_HINTS +from tests.support.upstream_cases import CASES_ROOT + +DEMO_VERIFIED = CASES_ROOT / "demo_verified" + + +def _event() -> ExternalChangeEvent: + return build_contract_change( + load_contract_file(DEMO_VERIFIED / "old.yaml"), load_contract_file(DEMO_VERIFIED / "new.yaml"), + hints=EMPTY_HINTS, + ) + + +def _patch(unified_diff: str = "diff") -> GeneratedPatch: + return GeneratedPatch( + origin=PatchOrigin.DETERMINISTIC, unified_diff=unified_diff, modified_files=("app/x.py",), + new_contents={"app/x.py": "content"}, step_results=(StepResult("s1", StepOutcome.APPLIED, "d"),), + safety=(SafetyGateResult("g", True, "ok"),), + ) + + +def _bundle(*, change_fingerprint: str, patch_fingerprint: str, repository_head_sha: str) -> MigrationEvidenceBundle: + plan = MigrationVerificationPlan(change_fingerprint, patch_fingerprint, (), (), ()) + coverage = VerificationCoverage((), (), (), (), ()) + return build_evidence_bundle( + plan=plan, repository="demo", repository_head_sha=repository_head_sha, step_evidence=(), contract_checks=(), + baseline_comparisons=(), coverage=coverage, evidence_strength=EvidenceStrength.NONE, + residual_risk=ResidualRisk.LOW, outcome=VerificationOutcome.VERIFIED, outcome_reasons=("ok",), + ) + + +def test_matching_evidence_passes() -> None: + event = _event() + patch = _patch() + bundle = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint=patch.fingerprint, repository_head_sha="deadbeef") + verify_evidence_integrity(event=event, patch=patch, bundle=bundle, base_commit_sha="deadbeef") + + +def test_mismatched_change_fingerprint_raises() -> None: + event = _event() + patch = _patch() + bundle = _bundle(change_fingerprint="wrong" * 12, patch_fingerprint=patch.fingerprint, repository_head_sha="deadbeef") + with pytest.raises(MigrationPRIntegrityError, match="upstream change"): + verify_evidence_integrity(event=event, patch=patch, bundle=bundle, base_commit_sha="deadbeef") + + +def test_mismatched_patch_fingerprint_raises() -> None: + event = _event() + patch = _patch() + bundle = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint="wrong" * 12, repository_head_sha="deadbeef") + with pytest.raises(MigrationPRIntegrityError, match="patch"): + verify_evidence_integrity(event=event, patch=patch, bundle=bundle, base_commit_sha="deadbeef") + + +def test_evidence_bundle_for_one_patch_is_never_reused_for_another() -> None: + event = _event() + patch_a = _patch("diff a") + patch_b = _patch("diff b") + assert patch_a.fingerprint != patch_b.fingerprint + bundle_for_a = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint=patch_a.fingerprint, repository_head_sha="deadbeef") + with pytest.raises(MigrationPRIntegrityError): + verify_evidence_integrity(event=event, patch=patch_b, bundle=bundle_for_a, base_commit_sha="deadbeef") + + +def test_mismatched_repository_head_sha_raises() -> None: + event = _event() + patch = _patch() + bundle = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint=patch.fingerprint, repository_head_sha="old-sha") + with pytest.raises(MigrationPRIntegrityError, match="repository head"): + verify_evidence_integrity(event=event, patch=patch, bundle=bundle, base_commit_sha="new-sha") + + +def test_no_patch_requires_empty_patch_fingerprint() -> None: + event = _event() + bundle = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint="", repository_head_sha="deadbeef") + verify_evidence_integrity(event=event, patch=None, bundle=bundle, base_commit_sha="deadbeef") + + +def test_no_patch_but_bundle_claims_a_patch_raises() -> None: + event = _event() + bundle = _bundle(change_fingerprint=event.fingerprint, patch_fingerprint="somepatch" * 8, repository_head_sha="deadbeef") + with pytest.raises(MigrationPRIntegrityError, match="patch"): + verify_evidence_integrity(event=event, patch=None, bundle=bundle, base_commit_sha="deadbeef") diff --git a/tests/unit/test_migration_verification_and_pr_module_boundaries.py b/tests/unit/test_migration_verification_and_pr_module_boundaries.py new file mode 100644 index 0000000..8c5eaea --- /dev/null +++ b/tests/unit/test_migration_verification_and_pr_module_boundaries.py @@ -0,0 +1,32 @@ +"""M8/M9's own structural provider-free invariant (mirrors +``test_deterministic_migration_modules_never_import_a_provider`` in +``tests/unit/test_migration_generator.py``): no module in +``patchfrog.migration_verification`` or ``patchfrog.migration_pr`` may +import an LLM provider abstraction or SDK. Neither package has any +optional model-assisted path (unlike ``patchfrog.migration.assist``), so +this is unconditional for every module in both packages.""" + +from __future__ import annotations + +import ast +from pathlib import Path + +_PACKAGE_DIRS = ( + Path(__file__).resolve().parent.parent.parent / "patchfrog" / "migration_verification", + Path(__file__).resolve().parent.parent.parent / "patchfrog" / "migration_pr", +) + + +def test_migration_verification_and_pr_modules_never_import_a_provider() -> None: + for package_dir in _PACKAGE_DIRS: + for path in package_dir.rglob("*.py"): + tree = ast.parse(path.read_text()) + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module: + assert not node.module.startswith(("patchfrog.review.provider", "patchfrog.review.providers")), ( + f"{path} imports {node.module}" + ) + if isinstance(node, ast.Import): + assert not any(a.name.split(".")[0] in ("anthropic", "openai", "google") for a in node.names), ( + f"{path} imports a provider SDK" + ) diff --git a/tests/unit/test_migration_verification_contract.py b/tests/unit/test_migration_verification_contract.py new file mode 100644 index 0000000..ace12dc --- /dev/null +++ b/tests/unit/test_migration_verification_contract.py @@ -0,0 +1,95 @@ +"""M8.7: deterministic contract verification, reusing M7's own +StepResult evidence rather than re-deriving it with string matching.""" + +from __future__ import annotations + +from patchfrog.migration.domain import ( + AutoFixEligibility, + EditOperation, + GeneratedPatch, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + PatchOrigin, + ResidualRisk, + SafetyGateResult, + StepOutcome, + StepResult, + VersionConstraint, +) +from patchfrog.migration_verification.contract import verify_contract +from patchfrog.migration_verification.domain import CheckStatus, VerificationRequirementKind +from patchfrog.migration_verification.requirements import generate_requirements + +_TARGET = MigrationTarget("repo", "app/chat.py", "generate_reply", 3, "chat.create", "sdk_call", "python") + + +def _step(strategy: MigrationStrategy, *, version_constraint: VersionConstraint | None = None) -> MigrationStep: + return MigrationStep("s1", _TARGET, strategy, AutoFixEligibility.AUTO_SAFE, ("d1",), ("k",), "o", "n", "n", (), + "", operation=EditOperation.of("op"), version_constraint=version_constraint) + + +def _plan(step: MigrationStep) -> MigrationPlan: + return MigrationPlan("fp", "repo", "sha", (), (step,), ResidualRisk.LOW, MigrationStatus.PLANNED) + + +def _patch(*, outcome: StepOutcome, new_contents: dict[str, str] | None = None) -> GeneratedPatch: + return GeneratedPatch( + origin=PatchOrigin.DETERMINISTIC, unified_diff="diff", modified_files=("app/chat.py",), + new_contents=new_contents or {"app/chat.py": "client.responses.create(...)"}, + step_results=(StepResult("s1", outcome, "detail", edits=1),), + safety=(SafetyGateResult("gate", True, "ok"),), + ) + + +def test_applied_step_passes_its_contract_requirements() -> None: + step = _step(MigrationStrategy.RENAME_SYMBOL) + plan = _plan(step) + requirements = generate_requirements(plan, test_selections=()) + checks = verify_contract(plan, _patch(outcome=StepOutcome.APPLIED), requirements) + obsolete = next(c for c in checks if "obsolete" in c.description) + assert obsolete.status is CheckStatus.PASSED + aggregate = next(c for c in checks if c.requirement_id == next( + r.requirement_id for r in requirements if r.kind is VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED + )) + assert aggregate.status is CheckStatus.PASSED + + +def test_failed_step_fails_its_contract_requirements_and_aggregate() -> None: + step = _step(MigrationStrategy.RENAME_SYMBOL) + plan = _plan(step) + requirements = generate_requirements(plan, test_selections=()) + checks = verify_contract(plan, _patch(outcome=StepOutcome.FAILED), requirements) + assert any(c.status is CheckStatus.FAILED for c in checks) + aggregate = next(c for c in checks if c.requirement_id == next( + r.requirement_id for r in requirements if r.kind is VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED + )) + assert aggregate.status is CheckStatus.FAILED + + +def test_version_constraint_mismatch_is_a_safety_net_failure() -> None: + constraint = VersionConstraint("requirements.txt", "acme-ai", "pypi", "1.4.0", "2.0.0") + step = _step(MigrationStrategy.BUMP_PACKAGE_VERSION, version_constraint=constraint) + plan = _plan(step) + requirements = generate_requirements(plan, test_selections=()) + # StepResult says APPLIED but the patched manifest doesn't actually + # contain the target version -- the generator's own claim alone is + # not proof; the content is re-checked. + patch = _patch(outcome=StepOutcome.APPLIED, new_contents={"app/chat.py": "acme-ai==1.9.0"}) + checks = verify_contract(plan, patch, requirements) + version_check = next(c for c in checks if "2.0.0" in c.description) + assert version_check.status is CheckStatus.FAILED + + +def test_missing_step_result_is_unavailable_not_a_guess() -> None: + step = _step(MigrationStrategy.RENAME_SYMBOL) + plan = _plan(step) + requirements = generate_requirements(plan, test_selections=()) + patch = GeneratedPatch( + origin=PatchOrigin.DETERMINISTIC, unified_diff="", modified_files=(), new_contents={}, + step_results=(), safety=(), + ) + checks = verify_contract(plan, patch, requirements) + assert any(c.status is CheckStatus.UNAVAILABLE for c in checks) diff --git a/tests/unit/test_migration_verification_decision.py b/tests/unit/test_migration_verification_decision.py new file mode 100644 index 0000000..b389909 --- /dev/null +++ b/tests/unit/test_migration_verification_decision.py @@ -0,0 +1,222 @@ +"""M8.11 / M8.14: the deterministic verification decision engine, covering +the milestone's own minimum fixture matrix at the decision-engine level +(pure, fast -- no sandbox needed). End-to-end sandboxed equivalents for +the baseline-evidence and human-required scenarios live in +``tests/integration/test_migration_verification_service.py`` against the +bundled demo repositories.""" + +from __future__ import annotations + +from patchfrog.migration_verification.decision import decide_outcome +from patchfrog.migration_verification.domain import ( + EvidenceStrength, + MigrationVerificationPlan, + VerificationCoverage, + VerificationOutcome, + VerificationRequirement, + VerificationRequirementKind, +) +from patchfrog.migration_verification.regression import RegressionFinding + +_REQ = VerificationRequirement( + requirement_id="r1", kind=VerificationRequirementKind.UNIT_TESTS_PASS, description="d", mandatory=True, +) +_REQ2 = VerificationRequirement( + requirement_id="r2", kind=VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED, description="d", + mandatory=True, +) +_PLAN = MigrationVerificationPlan( + change_fingerprint="c", patch_fingerprint="p", requirements=(_REQ, _REQ2), steps=(), test_selections=(), +) +_EMPTY_PLAN = MigrationVerificationPlan( + change_fingerprint="c", patch_fingerprint="p", requirements=(), steps=(), test_selections=(), +) + + +def _coverage(*, satisfied: tuple[str, ...] = (), failed: tuple[str, ...] = (), not_run: tuple[str, ...] = (), + unavailable: tuple[str, ...] = ()) -> VerificationCoverage: + return VerificationCoverage( + mandatory_requirement_ids=("r1", "r2"), satisfied_requirement_ids=satisfied, failed_requirement_ids=failed, + not_run_requirement_ids=not_run, unavailable_requirement_ids=unavailable, + ) + + +def test_baseline_fail_patched_pass_with_no_unresolved_reaches_verified() -> None: + coverage = _coverage(satisfied=("r1", "r2")) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.STRONG, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.VERIFIED + assert reasons + + +def test_baseline_pass_patched_pass_is_moderate_and_still_verified() -> None: + coverage = _coverage(satisfied=("r1", "r2")) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.MODERATE, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.VERIFIED + + +def test_patched_syntax_regression_is_regression_detected_even_with_other_evidence_satisfied() -> None: + coverage = _coverage(satisfied=("r1", "r2")) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.STRONG, + regressions=(RegressionFinding("syntax_failure", "py_compile failed"),), has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.REGRESSION_DETECTED + assert "syntax_failure" in reasons[0] + + +def test_targeted_test_regression_baseline_pass_patched_fail() -> None: + coverage = _coverage(satisfied=("r2",), failed=("r1",)) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.WEAK, + regressions=(RegressionFinding("targeted_test_regression", "baseline passed, patched failed"),), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.REGRESSION_DETECTED + + +def test_required_test_unavailable_is_unverified_or_partial_never_verified() -> None: + # No other satisfied evidence at all -> insufficient evidence, UNVERIFIED. + coverage = _coverage(unavailable=("r1", "r2")) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.NONE, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.UNVERIFIED + + +def test_contract_restored_without_runtime_test_is_partially_verified() -> None: + # Contract check passed (r2), but the unit-test requirement (r1) has + # no evidence at all -- strong contract evidence alone is never + # enough to reach VERIFIED without any executable evidence. + coverage = _coverage(satisfied=("r2",), unavailable=("r1",)) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.STRONG, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.PARTIALLY_VERIFIED + assert any("r1" in r for r in reasons) + + +def test_migration_partially_fixed_mandatory_requirement_failed_is_failed_outcome() -> None: + coverage = _coverage(satisfied=("r1",), failed=("r2",)) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.MODERATE, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.FAILED + assert any("r2" in r for r in reasons) + + +def test_human_required_migration_with_no_automatic_requirements() -> None: + outcome, reasons = decide_outcome( + _EMPTY_PLAN, coverage=_coverage(), evidence_strength=EvidenceStrength.NONE, regressions=(), + has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.HUMAN_REQUIRED + assert reasons + + +def test_human_required_when_automatic_part_verified_but_other_steps_remain() -> None: + coverage = _coverage(satisfied=("r1", "r2")) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.STRONG, regressions=(), + has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.HUMAN_REQUIRED + + +def test_unrelated_test_trap_never_forces_failed_when_only_weak_evidence_is_unresolved() -> None: + # A FALLBACK/non-blocking test's failure never enters `failed` in + # coverage (see evidence.build_coverage) -- it shows up as an + # unresolved (not_run) mandatory requirement instead, which is at + # worst PARTIALLY_VERIFIED/UNVERIFIED, never a false FAILED. + coverage = _coverage(satisfied=("r2",), not_run=("r1",)) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.WEAK, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome in (VerificationOutcome.PARTIALLY_VERIFIED, VerificationOutcome.UNVERIFIED) + + +def test_dependency_conflict_is_contract_mismatch_regression() -> None: + coverage = _coverage(satisfied=("r1",), failed=("r2",)) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.MODERATE, + regressions=(RegressionFinding("contract_mismatch", "acme-ai constraint does not allow 2.0.0"),), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.REGRESSION_DETECTED + assert "contract_mismatch" in reasons[0] + + +def test_no_requirements_and_no_human_steps_is_unverified_not_verified() -> None: + # A stale/empty plan (e.g. re-verifying against a repository state + # that no longer matches the patch) must never default to VERIFIED. + outcome, reasons = decide_outcome( + _EMPTY_PLAN, coverage=_coverage(), evidence_strength=EvidenceStrength.NONE, regressions=(), + has_unresolved_human_steps=False, + ) + assert outcome is VerificationOutcome.UNVERIFIED + assert reasons + + +def test_human_required_dominates_partially_verified() -> None: + coverage = _coverage(satisfied=("r2",), unavailable=("r1",)) + outcome, reasons = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.STRONG, regressions=(), + has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.HUMAN_REQUIRED + assert any("r1" in r for r in reasons) + + +def test_human_required_dominates_unverified() -> None: + coverage = _coverage(unavailable=("r1", "r2")) + outcome, _ = decide_outcome( + _PLAN, coverage=coverage, evidence_strength=EvidenceStrength.NONE, regressions=(), + has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.HUMAN_REQUIRED + + +def test_human_required_dominates_weak_evidence_formal_satisfaction() -> None: + outcome, _ = decide_outcome( + _PLAN, coverage=_coverage(satisfied=("r1", "r2")), evidence_strength=EvidenceStrength.WEAK, + regressions=(), has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.HUMAN_REQUIRED + + +def test_failed_dominates_human_required() -> None: + outcome, _ = decide_outcome( + _PLAN, coverage=_coverage(satisfied=("r1",), failed=("r2",)), evidence_strength=EvidenceStrength.MODERATE, + regressions=(), has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.FAILED + + +def test_regression_dominates_failed_and_human_required() -> None: + outcome, _ = decide_outcome( + _PLAN, coverage=_coverage(failed=("r1",)), evidence_strength=EvidenceStrength.STRONG, + regressions=(RegressionFinding("syntax_failure", "x"),), has_unresolved_human_steps=True, + ) + assert outcome is VerificationOutcome.REGRESSION_DETECTED + + +def test_partially_verified_and_unverified_without_human_steps_unchanged() -> None: + partial, _ = decide_outcome( + _PLAN, coverage=_coverage(satisfied=("r2",), unavailable=("r1",)), evidence_strength=EvidenceStrength.STRONG, + regressions=(), has_unresolved_human_steps=False, + ) + unverified, _ = decide_outcome( + _PLAN, coverage=_coverage(unavailable=("r1", "r2")), evidence_strength=EvidenceStrength.NONE, + regressions=(), has_unresolved_human_steps=False, + ) + assert partial is VerificationOutcome.PARTIALLY_VERIFIED + assert unverified is VerificationOutcome.UNVERIFIED diff --git a/tests/unit/test_migration_verification_evidence.py b/tests/unit/test_migration_verification_evidence.py new file mode 100644 index 0000000..ce5680a --- /dev/null +++ b/tests/unit/test_migration_verification_evidence.py @@ -0,0 +1,73 @@ +"""M8.9 / M8.10: coverage aggregation and evidence-strength assignment.""" + +from __future__ import annotations + +from patchfrog.migration_verification.domain import ( + BaselineComparison, + BaselineComparisonOutcome, + CheckStatus, + ContractCheckResult, + EvidenceStrength, + MigrationVerificationPlan, + StepEvidence, + VerificationRequirement, + VerificationRequirementKind, + VerificationStep, + VerificationStepKind, +) +from patchfrog.migration_verification.evidence import build_coverage, compute_evidence_strength + +_UNIT_TESTS_REQ = VerificationRequirement("r1", VerificationRequirementKind.UNIT_TESTS_PASS, "d", mandatory=True) + + +def _plan_with_step(*, blocks_verified: bool) -> MigrationVerificationPlan: + step = VerificationStep( + "st1", VerificationStepKind.TARGETED_UNIT_TEST, ("r1",), ("python", "-m", "pytest", "tests/test_x.py"), + 30.0, "reason", blocks_verified, + ) + return MigrationVerificationPlan("c", "p", (_UNIT_TESTS_REQ,), (step,), ()) + + +def test_fallback_non_blocking_failure_does_not_become_a_failed_requirement() -> None: + plan = _plan_with_step(blocks_verified=False) + evidence = (StepEvidence("st1", VerificationStepKind.TARGETED_UNIT_TEST, ("r1",), CheckStatus.FAILED, 1, 5.0, + "", "", "unrelated pre-existing failure"),) + coverage = build_coverage(plan, step_evidence=evidence, contract_checks=()) + assert "r1" not in coverage.failed_requirement_ids + assert "r1" in coverage.not_run_requirement_ids + + +def test_direct_blocking_failure_does_become_a_failed_requirement() -> None: + plan = _plan_with_step(blocks_verified=True) + evidence = (StepEvidence("st1", VerificationStepKind.TARGETED_UNIT_TEST, ("r1",), CheckStatus.FAILED, 1, 5.0, + "", "", "genuine failure"),) + coverage = build_coverage(plan, step_evidence=evidence, contract_checks=()) + assert "r1" in coverage.failed_requirement_ids + assert "r1" in coverage.failed_mandatory + + +def test_evidence_strength_strong_from_baseline_fail_patched_pass() -> None: + comparisons = (BaselineComparison("r1", CheckStatus.FAILED, CheckStatus.PASSED, + BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_PASS, "d"),) + strength = compute_evidence_strength(step_evidence=(), contract_checks=(), baseline_comparisons=comparisons) + assert strength is EvidenceStrength.STRONG + + +def test_evidence_strength_strong_from_all_mandatory_contract_checks_passed() -> None: + checks = (ContractCheckResult("r1", "d", CheckStatus.PASSED, "d"),) + strength = compute_evidence_strength(step_evidence=(), contract_checks=checks, baseline_comparisons=()) + assert strength is EvidenceStrength.STRONG + + +def test_evidence_strength_none_when_nothing_gathered() -> None: + strength = compute_evidence_strength(step_evidence=(), contract_checks=(), baseline_comparisons=()) + assert strength is EvidenceStrength.NONE + + +def test_many_weak_signals_never_promote_to_strong() -> None: + evidence = tuple( + StepEvidence(f"st{i}", VerificationStepKind.TYPE_CHECK, (f"r{i}",), CheckStatus.PASSED, 0, 1.0, "", "", "ok") + for i in range(10) + ) + strength = compute_evidence_strength(step_evidence=evidence, contract_checks=(), baseline_comparisons=()) + assert strength is EvidenceStrength.WEAK diff --git a/tests/unit/test_migration_verification_regression.py b/tests/unit/test_migration_verification_regression.py new file mode 100644 index 0000000..04b7e92 --- /dev/null +++ b/tests/unit/test_migration_verification_regression.py @@ -0,0 +1,89 @@ +"""M8.8: regression detection from already-gathered evidence.""" + +from __future__ import annotations + +from patchfrog.migration.domain import ( + AutoFixEligibility, + EditOperation, + GeneratedPatch, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + PatchOrigin, + ResidualRisk, + SafetyGateResult, + StepOutcome, + StepResult, +) +from patchfrog.migration_verification.domain import ( + BaselineComparison, + BaselineComparisonOutcome, + CheckStatus, + StepEvidence, + VerificationStepKind, +) +from patchfrog.migration_verification.regression import detect_regressions + +_TARGET = MigrationTarget("repo", "app/chat.py", "generate_reply", 3, "chat.create", "sdk_call", "python") +_STEP = MigrationStep("s1", _TARGET, MigrationStrategy.RENAME_SYMBOL, AutoFixEligibility.AUTO_SAFE, (), (), "o", "n", + "n", (), "", operation=EditOperation.of("rename_symbol")) +_PLAN = MigrationPlan("fp", "repo", "sha", (), (_STEP,), ResidualRisk.LOW, MigrationStatus.PLANNED) + + +def _patch(*, safety_ok: bool = True, modified_files: tuple[str, ...] = ("app/chat.py",)) -> GeneratedPatch: + return GeneratedPatch( + origin=PatchOrigin.DETERMINISTIC, unified_diff="diff", modified_files=modified_files, + new_contents=dict.fromkeys(modified_files, "content"), + step_results=(StepResult("s1", StepOutcome.APPLIED, "applied", edits=1),), + safety=(SafetyGateResult("gate", safety_ok, "detail"),), + ) + + +def test_safety_gate_failure_is_a_regression() -> None: + findings = detect_regressions(_PLAN, _patch(safety_ok=False), step_evidence=(), contract_checks=(), baseline_comparisons=()) + assert any(f.reason_code == "safety_gate_regression" for f in findings) + + +def test_syntax_check_failure_is_a_regression() -> None: + evidence = (StepEvidence("st1", VerificationStepKind.SYNTAX_CHECK, ("r1",), CheckStatus.FAILED, 1, 5.0, "", "err", + "py_compile failed"),) + findings = detect_regressions(_PLAN, _patch(), step_evidence=evidence, contract_checks=(), baseline_comparisons=()) + assert any(f.reason_code == "syntax_failure" for f in findings) + + +def test_type_check_failure_alone_is_never_a_regression() -> None: + # See regression.py's own docstring: PatchFrog's sandbox cannot prove + # a type-check failure is newly introduced (no repo config/stubs) -- + # this must never independently trigger REGRESSION_DETECTED. + evidence = (StepEvidence("st2", VerificationStepKind.TYPE_CHECK, ("r2",), CheckStatus.FAILED, 1, 5.0, "", "err", + "mypy reported type errors"),) + findings = detect_regressions(_PLAN, _patch(), step_evidence=evidence, contract_checks=(), baseline_comparisons=()) + assert not any(f.reason_code == "type_failure" for f in findings) + assert findings == () + + +def test_baseline_pass_patched_fail_is_targeted_test_regression() -> None: + comparisons = (BaselineComparison("r1", CheckStatus.PASSED, CheckStatus.FAILED, + BaselineComparisonOutcome.BASELINE_PASS_PATCHED_FAIL, "detail"),) + findings = detect_regressions(_PLAN, _patch(), step_evidence=(), contract_checks=(), baseline_comparisons=comparisons) + assert any(f.reason_code == "targeted_test_regression" for f in findings) + + +def test_baseline_fail_patched_pass_is_never_a_regression() -> None: + comparisons = (BaselineComparison("r1", CheckStatus.FAILED, CheckStatus.PASSED, + BaselineComparisonOutcome.BASELINE_FAIL_PATCHED_PASS, "detail"),) + findings = detect_regressions(_PLAN, _patch(), step_evidence=(), contract_checks=(), baseline_comparisons=comparisons) + assert findings == () + + +def test_unrelated_file_modification_is_flagged() -> None: + patch = _patch(modified_files=("app/chat.py", "unrelated/other.py")) + findings = detect_regressions(_PLAN, patch, step_evidence=(), contract_checks=(), baseline_comparisons=()) + assert any(f.reason_code == "unrelated_file_modified" and "unrelated/other.py" in f.detail for f in findings) + + +def test_clean_evidence_has_no_regressions() -> None: + findings = detect_regressions(_PLAN, _patch(), step_evidence=(), contract_checks=(), baseline_comparisons=()) + assert findings == () diff --git a/tests/unit/test_migration_verification_requirements.py b/tests/unit/test_migration_verification_requirements.py new file mode 100644 index 0000000..db14953 --- /dev/null +++ b/tests/unit/test_migration_verification_requirements.py @@ -0,0 +1,77 @@ +"""M8.2: verification requirement generation from real migration steps.""" + +from __future__ import annotations + +from patchfrog.migration.domain import ( + AutoFixEligibility, + EditOperation, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + ResidualRisk, + VersionConstraint, +) +from patchfrog.migration_verification.domain import VerificationRequirementKind +from patchfrog.migration_verification.requirements import generate_requirements + + +def _step(strategy: MigrationStrategy, *, version_constraint: VersionConstraint | None = None) -> MigrationStep: + target = MigrationTarget("repo", "app/chat.py", "generate_reply", 3, "chat.create", "sdk_call", "python") + return MigrationStep( + "s1", target, strategy, AutoFixEligibility.AUTO_SAFE, ("d1",), ("sdk_symbol_renamed",), "old", "new", "new", + (), "", operation=EditOperation.of("rename_symbol"), version_constraint=version_constraint, + ) + + +def _plan(*steps: MigrationStep) -> MigrationPlan: + status = MigrationStatus.PLANNED if all(s.auto_fix for s in steps) else MigrationStatus.PARTIAL + return MigrationPlan("fp", "repo", "deadbeef", (), steps, ResidualRisk.LOW, status) + + +def test_no_automatic_steps_yields_no_requirements() -> None: + target = MigrationTarget("repo", "app/x.py", None, None, "x", "sdk_call", "python") + step = MigrationStep("s1", target, MigrationStrategy.MANUAL_REVIEW, AutoFixEligibility.HUMAN_REQUIRED, (), (), + "", "", "", (), "") + plan = MigrationPlan("fp", "repo", None, (), (step,), ResidualRisk.HIGH, MigrationStatus.HUMAN_REQUIRED) + assert generate_requirements(plan, test_selections=()) == () + + +def test_rename_symbol_generates_obsolete_symbol_and_contract_requirements() -> None: + plan = _plan(_step(MigrationStrategy.RENAME_SYMBOL)) + requirements = generate_requirements(plan, test_selections=()) + kinds = {r.kind for r in requirements} + assert VerificationRequirementKind.OBSOLETE_SYMBOL_REMOVED in kinds + assert VerificationRequirementKind.SDK_CALL_MATCHES_CONTRACT in kinds + assert VerificationRequirementKind.CONTRACT_COMPATIBILITY_RESTORED in kinds + assert VerificationRequirementKind.SYNTAX_VALID in kinds + assert VerificationRequirementKind.UNIT_TESTS_PASS in kinds + obsolete = next(r for r in requirements if r.kind is VerificationRequirementKind.OBSOLETE_SYMBOL_REMOVED) + assert obsolete.source_step_id == "s1" + assert obsolete.mandatory + + +def test_syntax_is_mandatory_imports_and_typecheck_are_not() -> None: + plan = _plan(_step(MigrationStrategy.RENAME_SYMBOL)) + requirements = generate_requirements(plan, test_selections=()) + by_kind = {r.kind: r for r in requirements} + assert by_kind[VerificationRequirementKind.SYNTAX_VALID].mandatory + assert not by_kind[VerificationRequirementKind.IMPORTS_RESOLVE].mandatory + assert not by_kind[VerificationRequirementKind.TYPE_CHECK_PASSES].mandatory + assert not by_kind[VerificationRequirementKind.CONSUMER_BEHAVIOR_PRESERVED].mandatory + + +def test_add_required_parameter_generates_required_argument_present() -> None: + plan = _plan(_step(MigrationStrategy.ADD_REQUIRED_PARAMETER)) + requirements = generate_requirements(plan, test_selections=()) + assert any(r.kind is VerificationRequirementKind.REQUIRED_ARGUMENT_PRESENT for r in requirements) + + +def test_version_bump_generates_version_constraint_requirement() -> None: + constraint = VersionConstraint("requirements.txt", "acme-ai", "pypi", "1.4.0", "2.0.0") + plan = _plan(_step(MigrationStrategy.BUMP_PACKAGE_VERSION, version_constraint=constraint)) + requirements = generate_requirements(plan, test_selections=()) + version_req = next(r for r in requirements if r.kind is VerificationRequirementKind.VERSION_CONSTRAINT_COMPATIBLE) + assert version_req.mandatory + assert "2.0.0" in version_req.description diff --git a/tests/unit/test_migration_verification_test_selection.py b/tests/unit/test_migration_verification_test_selection.py new file mode 100644 index 0000000..d3fb10c --- /dev/null +++ b/tests/unit/test_migration_verification_test_selection.py @@ -0,0 +1,89 @@ +"""M8.3: deterministic targeted-test selection from real M6 BlastRadius +evidence -- never a filename-similarity guess.""" + +from __future__ import annotations + +from patchfrog.dependencies.domain import DetectionConfidence +from patchfrog.migration.domain import ( + AutoFixEligibility, + EditOperation, + MigrationPlan, + MigrationStatus, + MigrationStep, + MigrationStrategy, + MigrationTarget, + ResidualRisk, +) +from patchfrog.migration_verification.domain import MAX_TARGETED_TESTS_PER_PLAN, TestSelectionClass +from patchfrog.migration_verification.test_selection import select_targeted_tests +from patchfrog.upstream.blast_radius import BlastNode, BlastRadius, RelatedTest +from patchfrog.upstream.consumers import ImpactKind + + +def _automatic_plan() -> MigrationPlan: + target = MigrationTarget("repo", "app/chat.py", "generate_reply", 3, "chat.create", "sdk_call", "python") + step = MigrationStep( + "s1", target, MigrationStrategy.RENAME_SYMBOL, AutoFixEligibility.AUTO_SAFE, (), (), "old", "new", "new", + (), "", operation=EditOperation.of("rename_symbol"), + ) + return MigrationPlan("fp", "repo", None, (), (step,), ResidualRisk.LOW, MigrationStatus.PLANNED) + + +def _radius(related_tests: tuple[RelatedTest, ...], *, with_direct_node: bool = True) -> BlastRadius: + nodes = ( + (BlastNode("app/chat.py", "generate_reply", ImpactKind.DIRECT, DetectionConfidence.HIGH, 0, False, ()),) + if with_direct_node else () + ) + return BlastRadius( + repository="repo", dependency_key="acme-ai:pypi", nodes=nodes, edges=(), direct_site_keys=(), + related_tests=related_tests, modules=(), direct_callees=(), files_without_call_graph=(), graph_languages=(), + ) + + +def test_direct_high_confidence_test_is_classified_direct() -> None: + radius = _radius((RelatedTest("tests/test_chat.py", "app/chat.py::generate_reply", + "test consumes the changed contract directly", DetectionConfidence.HIGH),)) + plan = _automatic_plan() + selections = select_targeted_tests(plan, (radius,)) + assert len(selections) == 1 + assert selections[0].test_path == "tests/test_chat.py" + assert selections[0].classification is TestSelectionClass.DIRECT + + +def test_medium_confidence_caller_test_is_transitive() -> None: + radius = _radius((RelatedTest("tests/test_workers.py", "app/chat.py::generate_reply", + "test calls an affected symbol", DetectionConfidence.MEDIUM),)) + selections = select_targeted_tests(_automatic_plan(), (radius,)) + assert selections[0].classification is TestSelectionClass.TRANSITIVE + + +def test_low_confidence_evidence_is_fallback_never_invented() -> None: + radius = _radius((RelatedTest("tests/test_module.py", "app/chat.py", + "tests app/chat.py (filename convention)", DetectionConfidence.LOW),)) + selections = select_targeted_tests(_automatic_plan(), (radius,)) + assert selections[0].classification is TestSelectionClass.FALLBACK + + +def test_no_related_tests_at_all_yields_no_selections() -> None: + selections = select_targeted_tests(_automatic_plan(), (_radius(()),)) + assert selections == () + + +def test_no_automatic_steps_yields_no_selections_even_with_evidence() -> None: + target = MigrationTarget("repo", "app/x.py", None, None, "x", "sdk_call", "python") + step = MigrationStep("s1", target, MigrationStrategy.MANUAL_REVIEW, AutoFixEligibility.HUMAN_REQUIRED, (), (), + "", "", "", (), "") + plan = MigrationPlan("fp", "repo", None, (), (step,), ResidualRisk.HIGH, MigrationStatus.HUMAN_REQUIRED) + radius = _radius((RelatedTest("tests/test_x.py", "app/x.py", "test consumes the changed contract directly", + DetectionConfidence.HIGH),)) + assert select_targeted_tests(plan, (radius,)) == () + + +def test_selection_is_bounded() -> None: + related = tuple( + RelatedTest(f"tests/test_{i}.py", "app/chat.py::generate_reply", "test calls an affected symbol", + DetectionConfidence.MEDIUM) + for i in range(MAX_TARGETED_TESTS_PER_PLAN + 5) + ) + selections = select_targeted_tests(_automatic_plan(), (_radius(related),)) + assert len(selections) == MAX_TARGETED_TESTS_PER_PLAN diff --git a/tests/unit/test_review_checks.py b/tests/unit/test_review_checks.py index f478775..0fc2e29 100644 --- a/tests/unit/test_review_checks.py +++ b/tests/unit/test_review_checks.py @@ -20,7 +20,7 @@ class _FakeCheckClient: updates: list[tuple[int, GitHubCheckRunInput]] = field(default_factory=list) async def list_check_runs( - self, *, installation_id: int, ref: PullRequestRef, head_sha: str + self, *, installation_id: int, ref: PullRequestRef, head_sha: str, check_name: str = "PatchFrog review" ) -> list[GitHubCheckRun]: return list(self.checks) diff --git a/validation/m8_m9_verification_migration_pr/latest-summary.md b/validation/m8_m9_verification_migration_pr/latest-summary.md new file mode 100644 index 0000000..51b88f1 --- /dev/null +++ b/validation/m8_m9_verification_migration_pr/latest-summary.md @@ -0,0 +1,300 @@ +# M8 (Verification Lab) + M9 (Evidence-Backed Migration PR) — implementation plan + +**Baseline commit**: `812ac5a` on `main` (M6+M7 merged, matches the task's +stated baseline exactly). **Feature branch**: `feat/m8-m9-verification-migration-pr`. + +Permanent invariant this milestone must not weaken: + + change -> impact -> evidence -> verification -> decision + +Never: `change -> model opinion -> PR`. + +## Phase 0 audit — what already exists (reuse, not rebuild) + +Full detail lives in the PR description / final report; summary of what's +directly reused vs. genuinely new: + +**Reused as-is (no changes needed):** +- `patchfrog.executable_verification.sandbox.VerificationSandbox` (bwrap + + prlimit isolation) and `patchfrog.analysis.subprocess_sandbox.run_sandboxed` + — the complete M8.5 safe-execution boundary already exists. Every new + verification step routes through this, never a new subprocess call site. +- `patchfrog.executable_verification.snapshot_staging.export_artifact` / + `compute_artifact_digest` — credential-free artifact export + content + digest, reused for M8.6 baseline-vs-patched (export baseline commit, + build patched workspace, run the same adapter against both). +- `patchfrog.test_intelligence.expectations.derive_test_surfaces` — the + real deterministic test-to-symbol mapping, reused for M8.3 alongside + `BlastRadius.related_tests` (M6, already computed per migration). +- `patchfrog.upstream.code_graph` / `calls.py` — caller graph for M8.3's + transitive test/consumer reasoning. +- `patchfrog.migration.domain.PatchLinkage` — already documented as "what + M8 verification and M9 PR idempotency key on"; consumed unmodified. +- `patchfrog.repository.ancestry.verify_ancestor_with_diff` — reused for + M9.7's stale-base-before-publish check. +- `patchfrog.publishing.marker` pattern (HTML-comment marker + regex + extraction + `sanitize_untrusted_text`) — mirrored for the migration PR + marker. +- `patchfrog.publishing.service.ReviewPublicationService`'s durable + publish sequence (commit `PUBLISHING` row -> GitHub write with no lock + held -> re-lock and finalize; `RECONCILE_NEEDED` by marker; final + head-SHA race check) — the exact shape `MigrationPRPublisher` follows. +- `patchfrog.publishing.checks.ReviewCheckPublisher` / `CheckRunClient` + Protocol — reused for the new `MigrationCheckPublisher` (own check name, + own external-id scheme, same reconcile-by-external-id logic). +- `patchfrog.migration.store.MigrationStore`'s pattern (advisory lock, + fingerprint-keyed upsert, `_bounded()` JSON columns, `MAX_JSON_COLUMN_BYTES`) + — mirrored exactly for the new M8/M9 tables. +- `patchfrog.github.auth.InstallationTokenProvider` — unchanged. + +**Genuinely new (does not exist today):** +- The entire M8 domain model and decision engine (`patchfrog.migration_verification`). +- New safe adapters (syntax/import/type-check) beyond the existing pytest + adapter, all built on `VerificationSandbox` (same shape as `pytest_adapter.py`). +- `GitHubClient` has **no branch/ref/commit/tree creation and no + pull-request creation/list/update at all** — M9.2/M9.6/M9.9 require real + new methods on `patchfrog.github.client.GitHubClient`. +- M9 domain model, eligibility policy, branch naming, dossier rendering, + idempotent PR store (`patchfrog.migration_pr`). +- Two new bounded Alembic tables (verification runs, migration PRs), + chained after `0035_upstream_change_migration`. +- CLI: `patchfrog migrations verify` (M8.13) and + `patchfrog migrations publish --dry-run` (M9.10), added to + `patchfrog/cli_changes.py` alongside the existing `plan`/`generate`/`demo`. + +**Explicitly out of scope / not duplicated:** +- `patchfrog.fix_verification` (finding-centric, keyed by `handoff_id`) is + *not* extended or reused directly — only its evidence-combination + discipline (never let weak signals alone reach a terminal verdict; + prefer an honest non-answer) is mirrored in the new, migration-specific + decision engine. Separate enums, separate module, per M8.1's own + instruction not to overload existing review status enums. +- `patchfrog.mcp` / `patchfrog.agent_handoff` (external-agent fix-handoff + loop) is unrelated to M8/M9 (PatchFrog verifying its own generated + patch) and is not touched. +- No live provider call anywhere in M8/M9. `migration.assist`'s + model-assisted patch path already exists from M7 and is left exactly as + is — never wired into the verification/decision/publication path. +- `patchfrog-cloud` is not touched. Nothing in M8/M9 is a hosted-business + concern: verification and publication policy determine how PatchFrog + judges and acts on a change, which is engine territory per + `docs/product-boundary.md`. + +## Package layout + +``` +patchfrog/migration_verification/ # M8 + domain.py # M8.1 typed model + MIGRATION_VERIFICATION_VERSION + requirements.py # M8.2 requirement generation + test_selection.py # M8.3 targeted test selection (DIRECT/TRANSITIVE/FALLBACK/UNKNOWN) + adapters/ + syntax.py # python -m py_compile / ast.parse via sandbox + imports.py # import-resolution check via sandbox + mypy_adapter.py # mypy --strict via sandbox (only if repo has mypy configured) + execution_plan.py # M8.4 bounded VerificationStep sequence + baseline.py # M8.6 baseline-vs-patched two-state comparison + contract.py # M8.7 deterministic contract verification (reuses M6 DiffItemKind) + regression.py # M8.8 regression detection + evidence.py # M8.9/M8.10 MigrationEvidenceBundle + EvidenceStrength + decision.py # M8.11 deterministic outcome decision engine + service.py # orchestrates the whole M8 pipeline end to end + store.py # M8 persistence (mirrors migration/store.py) + policy.py # bounded constants + +patchfrog/migration_pr/ # M9 + domain.py # M9.1 typed model + MIGRATION_PR_VERSION + branch.py # M9.2 deterministic branch naming + eligibility.py # M9.3 publication eligibility policy + dossier.py # M9.4 PR body / Change Dossier rendering + check.py # M9.5 MigrationCheckPublisher (mirrors publishing/checks.py) + publisher.py # M9.6/M9.7/M9.8 durable publish sequence + integrity checks + store.py # M9 persistence (idempotency) + fake_github.py # M9.9 fake adapter for tests + +patchfrog/github/client.py # + create_ref/get_ref, create_blob/tree/commit, + # update_ref, create/list/update pull request +migrations/versions/0036_migration_verification_pr.py +patchfrog/cli_changes.py # + `migrations verify`, `migrations publish` +docs/migration-verification.md # M8 +docs/migration-pr.md # M9 +``` + +## Decision-engine discipline (mirrors fix_verification's corrected rule) + +`fix_verification`'s `FixEvidenceDirection` combination rule went through +two real false-positive/false-negative correction rounds (see +`validation/agent_handoff/latest-summary.md`). M8.11 adopts the same +governing principle from day one rather than re-discovering it: any single +strong contradicting signal (regression, contract check failing) wins +outright; only strong, direct evidence (baseline-fail-patched-pass on a +directly related check, or a direct contract-restoration proof) may reach +`VERIFIED`; any combination of only weak/generic evidence never promotes +above `PARTIALLY_VERIFIED`; missing required evidence is `UNVERIFIED`, not +guessed in either direction. + +## Commit plan (mirrors the suggested 5–8 logical commits) + +1. M8 verification domain + requirement generation + targeted test selection +2. M8 safe adapters (syntax/import/mypy) + execution plan + baseline/patched comparison +3. M8 contract verification + regression detection + evidence bundle + decision engine +4. M8 persistence (Alembic) + CLI `migrations verify` + M8 fixtures +5. M9 domain + branch naming + eligibility policy + dossier rendering +6. M9 GitHub adapter extension (branch/commit/tree/PR) + publisher + check publisher + fake adapter + persistence (Alembic) +7. M9 CLI `migrations publish --dry-run` + M8/M9 fixtures + full product demo extension +8. docs + this validation summary's final report section + +One PR opened at the end against `main`. Not merged automatically. + +## Validation gates before opening the PR + +ruff, mypy --strict, full pytest (Postgres), Alembic one-head check, fresh +Postgres migration, Docker builds, Celery registration (unaffected — no new +Celery tasks in this milestone), M4 cost benchmark, M5/M6/M7 suites, new +M8/M9 suites. No live provider calls, no live GitHub mutations in automated +tests. + +## Final report + +**Commits** (feature branch `feat/m8-m9-verification-migration-pr`, base `812ac5a`): + +| SHA | Summary | +|---|---| +| `175ffc3` | M8: verification domain, requirement generation, targeted test selection, safe adapters, execution plan, baseline/patched comparison, contract verification, regression detection, evidence bundle, decision engine | +| `698d9a3` | M8.13: `patchfrog migrations verify` CLI + persistence | +| `b72f0e4` | M9: `GitHubClient` branch/commit/tree creation + PR open/list/update | +| `af94df9` | M9: PR domain model, branch naming, eligibility policy, dossier, GitHub Check integration, durable idempotent/stale-base-protected publisher, M9.8 integrity check, persistence, fake-adapter tests | +| `aa70f30` | M9.10/M9.11/M9.12: `migrations publish --dry-run` CLI, opt-in (unimplemented, gated) real-GitHub E2E harness, full VERIFIED + HUMAN_REQUIRED product demo, docs | +| `f97d301` | docs: mark M8/M9 implemented in the roadmap | + +**PR**: opened against `main`, not merged (see PR description for the URL). + +**M8 architecture, reused primitives, requirement generation, targeted +test selection, sandbox/execution model, baseline-vs-patched, contract +verification, regression detection, evidence bundle structure, outcome +logic, evidence strength model, persistence** — all as designed in the +"Phase 0 audit" and "Package layout" sections above; no deviation was +needed during implementation. `MigrationVerificationRunModel` +(`0036_migration_verification`) persists one bounded row per (patch, +bundle fingerprint). + +**M9 publication eligibility, PR idempotency, stale-base/evidence +protection, GitHub Check integration** — as designed above +(`MigrationPRLinkage.identity_key()`, `MigrationPRStatus`, +`MigrationCheckPublisher`). One correction made during implementation: +the interrupted prior session had left a genuinely broken integration +test (`tests/integration/test_migration_pr_publisher.py` referenced a +nonexistent `sqlite_session_factory` fixture instead of +`tests/integration/conftest.py`'s real `session_factory`) and one +incorrect test expectation (a GitHub check run is inherently per-commit- +SHA; a regenerated patch that force-moves the branch to a new commit +*correctly* gets a second check run on its new SHA rather than +"reconciling" one that belongs to a commit the branch no longer points +at — the test was rewritten to cover both the true reconciliation case +(same commit, retried) and the true new-check-run case (different +commit) separately). Both are fixed in `af94df9`. + +**M9.8 (evidence/patch integrity)** was the one genuinely missing piece +from the interrupted session: `MigrationEvidenceBundle.bundle_fingerprint` +already existed as a hash tying evidence to an exact patch/change/repo +state, but nothing actually *checked* it against the patch/event about to +be published. Added `patchfrog/migration_pr/integrity.py` +(`verify_evidence_integrity`), called unconditionally at the top of +`build_pr_plan` — raises `MigrationPRIntegrityError` before any plan is +even built if the bundle's change/patch/repository-head fingerprints +don't correspond exactly. Covered by 7 unit tests including "a `VERIFIED` +bundle for patch A is never reused for patch B." + +**Dry-run PR dossier example** (`patchfrog migrations demo --verified`, +full text in the PR description): outcome `VERIFIED`, evidence strength +`strong` (real `baseline_fail_patched_pass` via the bundled `acme_ai` +2.0-shaped SDK stub), eligibility `AUTO_OPEN`, branch +`patchfrog/migrate/acme-ai/9cd9388cf635`, complete Change Dossier body +with Upstream change / Impact / Migration / Verification / Residual risk +/ Evidence identity sections and the trailing identity marker. + +**Complete end-to-end demo result**: `migrations demo --verified` reaches +`VERIFIED` / `AUTO_OPEN` — upstream change → affected consumer → blast +radius → migration plan → generated patch → bounded sandboxed +verification (syntax/import/type checks, 1/1 targeted test, 6/6 contract +checks, real baseline-fail→patched-pass) → evidence bundle → dry-run PR +dossier, exactly the M9.12 target flow. + +**Non-verified/human-required demo result**: the default `migrations +demo` (unchanged fixture, now extended through M8/M9) reaches +`HUMAN_REQUIRED` / `PLAN_ONLY` — one automatable-but-imperfect step +(`workers/summary.py::summarize`'s removed `stream` argument) correctly +blocks `VERIFIED` regardless of how cleanly everything else checks out +(10/10 contract checks, 2/2 targeted tests, real baseline evidence); no +code PR is produced, only a plan. + +**Full validation results**: + +| Gate | Result | +|---|---| +| ruff (repo-wide) | clean | +| mypy --strict (repo-wide) | clean, 465 source files | +| M8 suite (`test_migration_verification_*`) | 49 passed | +| M9 suites: `test_migration_pr_branch_eligibility` / `test_migration_pr_integrity` / `test_migration_pr_publisher` / `test_migration_publish_cli` / `test_migration_verification_and_pr_module_boundaries` | 10 + 7 + 13 + 5 + 1 = 36 passed | +| `test_migration_cli.py` (M7.10 demo + M9.12 `--verified` demo) | 9 passed | +| M4 cost benchmark | identical to the pre-M8/M9 baseline: 23→10 calls, 42930→20383 tokens, $0.046218→$0.022343, 2/2 findings preserved, all targets met | +| M5 dependency discovery suite | 35 passed | +| M6 suite (`test_upstream_consumers`, `test_upstream_contract_diff`, `test_upstream_cli`, `test_upstream_impact_corpus`, `test_upstream_store`) | 83 passed | +| M7 suite (`test_migration_assist`, `test_migration_generator`, `test_migration_planner`, `test_migration_corpus`, `test_migration_store`) | 67 passed, 5 skipped (expected — no-op materialization cases) | +| Alembic | single head (`0037_migration_pull_requests`); fresh `alembic upgrade head` from scratch on real Postgres succeeds through all 37 revisions | +| Docker build, `api`/`worker`/`verifier` targets | all three succeed; `patchfrog.migration_pr`/`patchfrog.migration_verification` confirmed importable inside the built `api` image | +| Celery task registration | 2/2 registration tests passed (unaffected — no new Celery task in this milestone) | +| Secret-pattern scan, branch diff (`main...HEAD`) + all new files | no private-key blocks, no AWS/GitHub/Slack/Google token-shaped strings, no credential-shaped assignments. **Scope note: this proves tracked-file/diff content only, not absence of terminal/UI/local display exposure.** | +| Full repo-wide pytest (all ~2800+ tests, real Postgres) | **not completed** — the harness OOM-killed the background run partway through due to genuine host memory pressure (347MB free, 5.9/9.6GB swap in use at the time), unrelated to this milestone's code. Every suite this milestone actually touches was independently run to completion instead (rows above); the operator was asked and chose to proceed to PR on that basis rather than retry locally. | + +**M4–M7 regression status**: no regression in any of the above — M4's +cost benchmark table is byte-for-byte identical to the pre-M8/M9 +baseline; M5/M6/M7 suites all pass at their expected counts. + +**Known limitations** (also documented in `docs/migration-verification.md` +and `docs/migration-pr.md`): + +- Baseline-vs-patched comparisons are bounded to 2 per verification run, + `DIRECT`/`TRANSITIVE` selections only. +- Where the verification sandbox itself is unavailable on a host, every + step is honestly reported `UNAVAILABLE` rather than silently skipped or + guessed passing. +- Real GitHub migration-PR publication (`GitHubClientMigrationPublisher`) + is wired but not yet called from the production webhook/worker + pipeline — the CLI intentionally exposes only `--dry-run`, and the + opt-in real-GitHub E2E harness (M9.11) is deliberately left + unimplemented and never runs automatically. +- The repo-wide full pytest run could not be completed locally this + session due to host memory pressure (see the validation table above); + CI should be treated as the authoritative full-suite confirmation for + this PR. +- The tracked-file secret scan (this milestone and every prior one) never + proves absence of terminal/UI/local display exposure, only tracked-file + content. + +**Milestone completion**: + +- **M8 is complete** for its stated scope (all 14 sub-sections, M8.1 + through M8.14 — the 14-fixture-scenario intent of M8.14 is satisfied + across the two bundled end-to-end demo fixtures plus the focused + decision-engine/regression-detection unit suites, which enumerate the + distinct outcome scenarios directly rather than each needing its own + full repository fixture). +- **M9 is complete** for its stated scope (M9.1 through M9.12, including + the previously-missing M9.8 integrity check, M9.10 CLI, M9.11 opt-in + harness stub, and M9.12 full demo). +- **PatchFrog now has a complete first version of:** external API/SDK + change → deterministic impact → generated patch → bounded executable + verification → evidence-backed, policy-gated migration PR dry-run, + end to end, entirely offline and provider-free. +- **What remains before M10/M11** (cross-org scale, continuous watchers, + Cloud productization, production beta): wiring + `GitHubClientMigrationPublisher`/`MigrationPRPublisher` into the actual + production webhook/worker pipeline (currently only reachable via the + local CLI dry-run and the fake-adapter test suite); a scheduled/ + triggered upstream-change watcher (this milestone only reacts to a + change given on the command line, never polls a real registry/release + feed on its own); real GitHub App credentials and an actual opt-in + M9.11 run against a disposable fixture repository; and, per + `docs/product-boundary.md`, any hosted-business concern (accounts, + billing, provider routing policy, multi-tenant scale) belongs to + `patchfrog-cloud`, not this repository.