diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d3fda9..b5ed16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes. ## 2026-07-21 — Consolidated carve-changesets CLI and live contract +- feat: add stateless changeset merge and propagation - refactor: make strict apply use one proof + (`c8ca89566562d7d154bfe1a1711140323e3ba9f8`) - fix: bind GitHub operations to the selected remote (`cfdddb0aeb792fabfb4021173e25738b45329083`) - fix: close consolidated carve CLI review gaps diff --git a/skills/carve-changesets/scripts/cli.py b/skills/carve-changesets/scripts/cli.py index e951720..faf3ab3 100755 --- a/skills/carve-changesets/scripts/cli.py +++ b/skills/carve-changesets/scripts/cli.py @@ -22,7 +22,7 @@ from patch_apply import build_diff from plan_checks import strict_apply_check, validate_plan_strict from preflight import preflight -from propagate import push_chain +from propagate import merge_propagate_from_live, propagate_from_live, push_chain from rehydrate import RehydrationError, discover_changeset_heads, rehydrate_chain from squash_check import squash_check from squash_ref import _resolve_base_source, create_squashed_ref @@ -43,6 +43,8 @@ "validate-chain": LOCAL_MUTATING, "pr-create": REMOTE_MUTATING, "push-chain": REMOTE_MUTATING, + "propagate": REMOTE_MUTATING, + "merge-propagate": REMOTE_MUTATING, "db-compare": LOCAL_MUTATING, "hunk-preview": READ_ONLY, "squash-ref": LOCAL_MUTATING, @@ -213,6 +215,33 @@ def cmd_push_chain(args: argparse.Namespace) -> None: ) +def cmd_propagate(args: argparse.Namespace) -> None: + propagate_from_live( + source=args.source, + base=args.base, + pr_number=args.pr, + index=args.index, + strategy=args.strategy, + remote=args.remote, + dry_run=args.dry_run, + authority_acknowledged=args.ack_merge_and_propagate, + ) + + +def cmd_merge_propagate(args: argparse.Namespace) -> None: + merge_propagate_from_live( + source=args.source, + base=args.base, + pr_number=args.pr, + index=args.index, + strategy=args.strategy, + method=args.method, + remote=args.remote, + dry_run=args.dry_run, + authority_acknowledged=args.ack_merge_and_propagate, + ) + + def cmd_db_compare(args: argparse.Namespace) -> None: db_compare( load_and_validate(Path(args.plan)), @@ -321,6 +350,24 @@ def _add_remote_dry_run(parser: argparse.ArgumentParser) -> None: parser.set_defaults(dry_run=True) +def _add_propagation_options(parser: argparse.ArgumentParser) -> None: + parser.add_argument("--source", required=True, help="Source branch") + parser.add_argument("--base", default=None, help="Base branch") + target = parser.add_mutually_exclusive_group(required=True) + target.add_argument("--pr", type=int, help="Changeset pull request number") + target.add_argument("--index", type=int, help="One-based changeset index") + parser.add_argument( + "--strategy", choices=("rebase", "cherry-pick"), default="rebase" + ) + parser.add_argument("--remote", default="origin") + parser.add_argument( + "--ack-merge-and-propagate", + action="store_true", + help="Acknowledge explicit merge-and-propagate authority", + ) + _add_remote_dry_run(parser) + + def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( description="Carve a review-ready source branch into intentional changesets.", @@ -388,6 +435,25 @@ def build_parser() -> argparse.ArgumentParser: _add_remote_dry_run(item) item.set_defaults(func=cmd_push_chain) + item = _command( + sub, + "propagate", + "Verify a merged changeset and propagate its downstream suffix.", + ) + _add_propagation_options(item) + item.set_defaults(func=cmd_propagate) + + item = _command( + sub, + "merge-propagate", + "Merge one changeset PR, verify it, and propagate downstream.", + ) + _add_propagation_options(item) + item.add_argument( + "--method", choices=("merge", "squash", "rebase"), default="merge" + ) + item.set_defaults(func=cmd_merge_propagate) + item = _command(sub, "db-compare", "Compare source and chain database schemas.") _add_plan(item) item.add_argument("--source-cmd", required=True) diff --git a/skills/carve-changesets/scripts/github.py b/skills/carve-changesets/scripts/github.py index 9434406..f4c50f4 100644 --- a/skills/carve-changesets/scripts/github.py +++ b/skills/carve-changesets/scripts/github.py @@ -27,6 +27,11 @@ ) from rehydrate import PullRequestRecord +_PR_JSON_FIELDS = ( + "number,headRefName,headRefOid,baseRefName,state,body,title,mergeCommit," + "isCrossRepository" +) + def _format_error(command: Sequence[str], error: subprocess.CalledProcessError) -> str: stdout = (error.stdout or "").strip() @@ -308,7 +313,7 @@ def pull_requests_for_source( "--limit", "100", "--json", - "number,headRefName,headRefOid,baseRefName,state,body", + _PR_JSON_FIELDS, ) ) if not isinstance(payload, list): @@ -322,14 +327,121 @@ def pull_requests_for_source( suffix = head.removeprefix(prefix) if not head.startswith(prefix) or not suffix.isdigit() or int(suffix) < 1: continue - records.append( - PullRequestRecord( - number=int(item["number"]), - head_branch=head, - head_sha=str(item.get("headRefOid") or ""), - base_branch=str(item.get("baseRefName") or ""), - state=str(item.get("state") or ""), - body=str(item.get("body") or ""), - ) - ) + records.append(_pull_request_record(item, context=f"changeset PR for {head}")) return records + + +def _merge_sha(item: Dict) -> str | None: + value = item.get("mergeCommit") + if not isinstance(value, dict): + return None + oid = str(value.get("oid") or "").strip() + return oid or None + + +def _pull_request_record(item: object, *, context: str) -> PullRequestRecord: + """Decode one selected gh PR payload with operation-specific errors.""" + + if not isinstance(item, dict): + raise CommandError(f"Unexpected GitHub response for {context}.") + try: + number = int(item["number"]) + except (KeyError, TypeError, ValueError) as exc: + raise CommandError( + f"GitHub response for {context} has no valid PR number." + ) from exc + return PullRequestRecord( + number=number, + head_branch=str(item.get("headRefName") or ""), + head_sha=str(item.get("headRefOid") or ""), + base_branch=str(item.get("baseRefName") or ""), + state=str(item.get("state") or ""), + body=str(item.get("body") or ""), + title=str(item.get("title") or ""), + merge_sha=_merge_sha(item), + is_cross_repository=bool(item.get("isCrossRepository", False)), + ) + + +def pull_request_by_number(number: int, *, remote: str = "origin") -> PullRequestRecord: + """Read one exact PR using the same fields as chain discovery.""" + + repository = github_repo_for_remote(remote) + item = gh_json( + ( + "pr", + "view", + str(number), + "-R", + repository, + "--json", + _PR_JSON_FIELDS, + ) + ) + record = _pull_request_record(item, context=f"PR #{number}") + actual_number = record.number + if actual_number != number: + raise CommandError( + f"GitHub returned PR #{actual_number} while verifying requested PR #{number}." + ) + return record + + +def merge_pull_request( + number: int, + *, + expected_head: str, + method: str, + remote: str = "origin", + dry_run: bool, +) -> None: + """Merge one explicitly numbered PR in the selected remote repository.""" + + method_flags = {"merge": "--merge", "squash": "--squash", "rebase": "--rebase"} + if method not in method_flags: + raise CommandError("Merge method must be 'merge', 'squash', or 'rebase'.") + repository = github_repo_for_remote(remote) + args = ( + "pr", + "merge", + str(number), + "-R", + repository, + method_flags[method], + "--match-head-commit", + expected_head, + ) + print(f"[STEP] Merging PR #{number} with method={method}") + if dry_run: + print("[DRY-RUN] Would run:") + _print_command(("gh", *args)) + return + ensure_gh_ready(repository) + gh_capture(args) + + +def edit_pull_request( + number: int, + *, + remote: str = "origin", + base: str | None = None, + title: str | None = None, + dry_run: bool, +) -> None: + """Edit one explicit PR; never infer a target from the checked-out branch.""" + + if base is None and title is None: + return + repository = github_repo_for_remote(remote) + args: list[str] = ["pr", "edit", str(number), "-R", repository] + if base is not None: + args.extend(("--base", base)) + if title is not None: + args.extend(("--title", title)) + print(f"[STEP] Updating PR #{number}") + if dry_run: + print("[DRY-RUN] Would run:") + _print_command(("gh", *args)) + return + ensure_gh_ready(repository) + gh_capture(tuple(args)) diff --git a/skills/carve-changesets/scripts/propagate.py b/skills/carve-changesets/scripts/propagate.py index 8a93f8c..0a74598 100644 --- a/skills/carve-changesets/scripts/propagate.py +++ b/skills/carve-changesets/scripts/propagate.py @@ -1,22 +1,33 @@ #!/usr/bin/env python3 -"""Remote push safety for a materialized changeset chain. - -Merge and downstream propagation belong to epic child #33. This module retains -only the #30 push-chain surface. -""" +"""Stateless merge, downstream propagation, and remote push safety.""" from __future__ import annotations +import re +from pathlib import Path from typing import Dict, List from common import ( CommandError, branch_exists, branch_name_for, + current_branch, ensure_clean_tree, ensure_git_repo, git, ) +from github import ( + edit_pull_request, + merge_pull_request, + pull_request_by_number, + pull_requests_for_source, +) +from metadata import MetadataError, parse_commit_message, parse_pr_metadata +from rehydrate import Chain, ChangesetRecord, PullRequestRecord, rehydrate_chain +from validate import validate_live_chain + +AUTHORITY_FLAG = "--ack-merge-and-propagate" +_TITLE_COUNT_RE = re.compile(r"\s+\([1-9][0-9]* of [1-9][0-9]*\)$") def _ensure_chain_exists(source: str, total: int) -> List[str]: @@ -45,8 +56,20 @@ def remote_branch_head(remote: str, branch: str) -> str | None: return line.split()[0] if line else None -def push_changeset_branch(branch: str, *, remote: str, dry_run: bool) -> None: - expected = remote_branch_head(remote, branch) +def push_changeset_branch( + branch: str, + *, + remote: str, + dry_run: bool, + expected_remote_head: str | None = None, +) -> None: + current = remote_branch_head(remote, branch) + if expected_remote_head is not None and current != expected_remote_head: + raise CommandError( + f"Remote branch {remote}/{branch} moved from verified head " + f"{expected_remote_head} to {current}; propagation was withheld." + ) + expected = expected_remote_head if expected_remote_head is not None else current lease = f"--force-with-lease=refs/heads/{branch}:{expected or ''}" refspec = f"refs/heads/{branch}:refs/heads/{branch}" command = ("git", "push", remote, refspec, lease) @@ -81,3 +104,602 @@ def push_chain(plan: Dict, *, remote: str, dry_run: bool) -> None: print("[OK] Dry-run push-chain complete. Re-run with --no-dry-run to execute.") else: print("[OK] push-chain completed.") + + +def _fetch_remote(remote: str) -> None: + if not remote_exists(remote): + raise CommandError(f"Remote does not exist: {remote}") + git("fetch", "--prune", remote) + + +def _remote_ref(remote: str, branch: str) -> str: + return f"refs/remotes/{remote}/{branch}" + + +def _resolve(ref: str) -> str: + result = git("rev-parse", "--verify", f"{ref}^{{commit}}", check=False) + if result.returncode != 0: + raise CommandError(f"Git commit is unavailable: {ref}") + return result.stdout.strip() + + +def _is_ancestor(ancestor: str, descendant: str) -> bool: + result = git("merge-base", "--is-ancestor", ancestor, descendant, check=False) + if result.returncode == 0: + return True + if result.returncode == 1: + return False + raise CommandError( + f"Git could not compare mainline commit {descendant} with merged commit {ancestor}." + ) + + +def _rehydrate_live( + *, source: str, base: str | None, remote: str +) -> tuple[Chain, dict[int, PullRequestRecord]]: + _fetch_remote(remote) + pull_requests = pull_requests_for_source(source, remote=remote) + for pr in pull_requests: + if pr.state.upper() != "MERGED": + continue + available = git("cat-file", "-e", f"{pr.head_sha}^{{commit}}", check=False) + if available.returncode == 0: + continue + fetched = git("fetch", remote, f"refs/pull/{pr.number}/head", check=False) + available = git("cat-file", "-e", f"{pr.head_sha}^{{commit}}", check=False) + if fetched.returncode != 0 or available.returncode != 0: + raise CommandError( + f"Merged PR #{pr.number} head {pr.head_sha} is unavailable; " + "fetch its exact GitHub PR head before retrying." + ) + chain = rehydrate_chain( + source_branch=source, + base_branch=base, + pull_requests=pull_requests, + cwd=Path.cwd(), + remote=remote, + prefer_remote=True, + ) + validation = validate_live_chain( + chain, + cwd=Path.cwd(), + remote=remote, + allow_partial_propagation=True, + ) + if not validation.valid: + detail = "; ".join( + f"{diagnostic.code}: {diagnostic.message}" + for diagnostic in validation.errors + ) + raise CommandError(f"Live changeset chain is invalid: {detail}") + by_number = {pr.number: pr for pr in pull_requests} + return chain, by_number + + +def _target( + chain: Chain, + pull_requests: dict[int, PullRequestRecord], + *, + pr_number: int | None, + index: int | None, +) -> tuple[ChangesetRecord, PullRequestRecord]: + if (pr_number is None) == (index is None): + raise CommandError("Pass exactly one of --pr or --index.") + record: ChangesetRecord | None = None + if index is not None: + if index < 1 or index > len(chain.changesets): + raise CommandError( + f"--index must be between 1 and {len(chain.changesets)}." + ) + record = chain.changesets[index - 1] + else: + record = next( + (item for item in chain.changesets if item.pr_number == pr_number), None + ) + if record is None: + raise CommandError( + f"PR #{pr_number} does not belong to this changeset chain." + ) + if record.pr_number is None or record.pr_number not in pull_requests: + raise CommandError( + f"Changeset {record.metadata.index} has no verified GitHub pull request." + ) + return record, pull_requests[record.pr_number] + + +def _require_sequential_target(chain: Chain, target_index: int) -> None: + for item in chain.changesets[: target_index - 1]: + if item.pr_state != "MERGED": + raise CommandError( + f"Changeset {target_index} cannot proceed before PR for changeset " + f"{item.metadata.index} is verified merged." + ) + for item in chain.changesets[target_index:]: + if item.pr_state == "MERGED": + raise CommandError( + f"Changeset {item.metadata.index} is merged out of sequence." + ) + if item.pr_state != "OPEN": + raise CommandError( + f"Downstream PR #{item.pr_number} must be OPEN, got {item.pr_state or 'missing'}." + ) + + +def _require_authority(*, dry_run: bool, authority_acknowledged: bool) -> None: + if not dry_run and not authority_acknowledged: + raise CommandError( + f"Remote execution requires {AUTHORITY_FLAG} in addition to --no-dry-run." + ) + + +def _verify_merged_on_base( + pr: PullRequestRecord, *, base: str, remote: str +) -> PullRequestRecord: + live = pull_request_by_number(pr.number, remote=remote) + if live.state.upper() != "MERGED": + raise CommandError( + f"PR #{pr.number} is {live.state or 'UNKNOWN'}, not verified MERGED; " + "downstream propagation was withheld." + ) + if live.head_branch != pr.head_branch: + raise CommandError( + f"PR #{pr.number} head branch changed from {pr.head_branch!r} " + f"to {live.head_branch!r}." + ) + if live.head_sha != pr.head_sha: + raise CommandError( + f"PR #{pr.number} head moved from verified commit {pr.head_sha} " + f"to {live.head_sha}." + ) + if not live.merge_sha: + raise CommandError( + f"PR #{pr.number} is merged but GitHub did not report a merge commit." + ) + _fetch_remote(remote) + base_head = _resolve(_remote_ref(remote, base)) + merge_head = _resolve(live.merge_sha) + if not _is_ancestor(merge_head, base_head): + raise CommandError( + f"PR #{pr.number} reports merge commit {merge_head}, but {remote}/{base} " + f"at {base_head} does not contain it." + ) + return live + + +def _ensure_local_branch(record: ChangesetRecord) -> None: + local = git( + "rev-parse", + "--verify", + f"refs/heads/{record.branch}^{{commit}}", + check=False, + ) + if local.returncode == 0: + if local.stdout.strip() != record.head: + raise CommandError( + f"Local branch {record.branch} moved from verified head {record.head} " + f"to {local.stdout.strip()}." + ) + return + git("branch", record.branch, record.head) + + +def _rewrite_rebase( + record: ChangesetRecord, *, old_base: str, new_base: str, dry_run: bool +) -> str: + print(f"[STEP] Rebasing {record.branch} onto {new_base}") + if dry_run: + print( + f"[DRY-RUN] Would run: git rebase --onto {new_base} {old_base} {record.branch}" + ) + return record.head + _ensure_local_branch(record) + git("rebase", "--onto", new_base, old_base, record.branch) + return _resolve(f"refs/heads/{record.branch}") + + +def _rewrite_cherry_pick( + record: ChangesetRecord, *, old_base: str, new_base: str, dry_run: bool +) -> str: + commits = [ + value + for value in git( + "rev-list", "--reverse", f"{old_base}..{record.head}" + ).stdout.splitlines() + if value + ] + if not commits: + raise CommandError( + f"Changeset branch {record.branch} has no commits beyond its verified predecessor." + ) + print( + f"[STEP] Cherry-picking {len(commits)} commit(s) for {record.branch} onto {new_base}" + ) + if dry_run: + print(f"[DRY-RUN] Would detach at {new_base} and cherry-pick exact commits.") + return record.head + _ensure_local_branch(record) + git("checkout", "--detach", new_base) + for commit in commits: + git("cherry-pick", commit) + rewritten = _resolve("HEAD") + git("update-ref", f"refs/heads/{record.branch}", rewritten, record.head) + git("checkout", record.branch) + return rewritten + + +def _updated_title(pr: PullRequestRecord, *, index: int, total: int) -> str: + current = pr.title.strip() + if not current: + raise CommandError(f"PR #{pr.number} has no title to update.") + prefix = _TITLE_COUNT_RE.sub("", current) + return f"{prefix} ({index} of {total})" + + +def _durable_predecessor(record: ChangesetRecord, previous: ChangesetRecord) -> str: + """Find the prior changeset commit in an unpropagated branch's ancestry.""" + + if _is_ancestor(previous.head, record.head): + return previous.head + for commit in git("rev-list", record.head).stdout.splitlines(): + message = git("show", "-s", "--format=%B", commit).stdout + try: + metadata = parse_commit_message(message) + except MetadataError: + continue + if ( + metadata.index == previous.metadata.index + and metadata.source_branch == record.metadata.source_branch + and metadata.source_sha == record.metadata.source_sha + ): + return commit + raise CommandError( + f"Changeset branch {record.branch} does not contain durable predecessor " + f"metadata for changeset {previous.metadata.index}." + ) + + +def _verify_live_downstream( + record: ChangesetRecord, + *, + expected_remote_head: str, + allowed_bases: set[str], + remote: str, + role: str = "Downstream", +) -> PullRequestRecord: + """Reauthorize one exact open downstream PR immediately before mutation.""" + + if record.pr_number is None: + raise CommandError( + f"Downstream changeset {record.metadata.index} has no verified PR." + ) + live = pull_request_by_number(record.pr_number, remote=remote) + if live.state.upper() != "OPEN": + raise CommandError( + f"{role} PR #{live.number} changed to {live.state or 'UNKNOWN'}; " + "remote mutation was withheld." + ) + if live.is_cross_repository: + raise CommandError( + f"{role} PR #{live.number} changed to a fork head; remote mutation " + "was withheld." + ) + if live.head_branch != record.branch: + raise CommandError( + f"{role} PR #{live.number} head branch changed from {record.branch!r} " + f"to {live.head_branch!r}; remote mutation was withheld." + ) + if live.head_sha != expected_remote_head: + raise CommandError( + f"{role} PR #{live.number} head moved from {expected_remote_head} " + f"to {live.head_sha}; remote mutation was withheld." + ) + if live.base_branch not in allowed_bases: + hint = ( + " Resume propagation for the preceding merged changeset first." + if role == "Merge target" + else "" + ) + raise CommandError( + f"{role} PR #{live.number} base changed from {record.base!r} to " + f"{live.base_branch!r}; expected one of " + f"{', '.join(repr(item) for item in sorted(allowed_bases))}. " + f"Remote mutation was withheld.{hint}" + ) + try: + live_metadata = parse_pr_metadata(live.body) + except MetadataError as exc: + raise CommandError( + f"{role} PR #{live.number} metadata is invalid: {exc}" + ) from exc + if live_metadata != record.metadata: + raise CommandError( + f"{role} PR #{live.number} no longer belongs to changeset " + f"{record.metadata.index}; remote mutation was withheld." + ) + remote_head = remote_branch_head(remote, record.branch) + if remote_head != expected_remote_head: + raise CommandError( + f"Remote branch {remote}/{record.branch} moved from verified head " + f"{expected_remote_head} to {remote_head}; remote mutation was withheld." + ) + return live + + +def _require_merge_target_ancestry( + record: ChangesetRecord, *, base: str, remote: str +) -> None: + predecessor = _resolve(_remote_ref(remote, base)) + if not _is_ancestor(predecessor, record.head): + raise CommandError( + f"PR #{record.pr_number} cannot merge: changeset branch {record.branch} " + f"does not descend from its live base {base}." + ) + + +def _verify_published_downstream( + pr: PullRequestRecord, + *, + expected_head: str, + expected_base: str, + expected_title: str, + remote: str, +) -> None: + remote_head = remote_branch_head(remote, pr.head_branch) + if remote_head != expected_head: + raise CommandError( + f"Remote branch {remote}/{pr.head_branch} is {remote_head}; " + f"expected propagated head {expected_head}." + ) + live = pull_request_by_number(pr.number, remote=remote) + if live.state.upper() != "OPEN": + raise CommandError( + f"Downstream PR #{pr.number} is {live.state or 'UNKNOWN'}, not OPEN." + ) + if live.head_sha != expected_head: + raise CommandError( + f"PR #{pr.number} head is {live.head_sha}; expected {expected_head}." + ) + if live.base_branch != expected_base: + raise CommandError( + f"PR #{pr.number} base is {live.base_branch!r}; expected {expected_base!r}." + ) + if live.title != expected_title: + raise CommandError( + f"PR #{pr.number} title is {live.title!r}; expected {expected_title!r}." + ) + + +def _propagate_chain( + chain: Chain, + pull_requests: dict[int, PullRequestRecord], + *, + merged_index: int, + strategy: str, + remote: str, + dry_run: bool, +) -> None: + if strategy not in ("rebase", "cherry-pick"): + raise CommandError("Propagation strategy must be 'rebase' or 'cherry-pick'.") + downstream = list(chain.changesets[merged_index:]) + if not downstream: + print("[OK] Merged changeset has no downstream branches to propagate.") + return + + planned: list[tuple[ChangesetRecord, PullRequestRecord, str]] = [] + for record in downstream: + pr_number = record.pr_number + if pr_number is None or pr_number not in pull_requests: + raise CommandError( + f"Downstream changeset {record.metadata.index} has no verified PR." + ) + pr = pull_requests[pr_number] + expected_base = ( + chain.base_branch + if record.metadata.index == merged_index + 1 + else chain.changesets[record.metadata.index - 2].branch + ) + planned.append((record, pr, expected_base)) + + original = current_branch() + new_base = _remote_ref(remote, chain.base_branch) + rewrite_frontier_reached = False + for record, _pr, expected_base in planned: + current_head = record.head + allowed_bases = {record.base} + if expected_base == chain.base_branch: + allowed_bases.add(chain.base_branch) + live = _verify_live_downstream( + record, + expected_remote_head=current_head, + allowed_bases=allowed_bases, + remote=remote, + ) + already_propagated = not rewrite_frontier_reached and _is_ancestor( + new_base, current_head + ) + if already_propagated: + new_head = current_head + print(f"[INFO] {record.branch} is already propagated; push not needed.") + else: + rewrite_frontier_reached = True + previous = chain.changesets[record.metadata.index - 2] + old_base = _durable_predecessor(record, previous) + if strategy == "rebase": + new_head = _rewrite_rebase( + record, old_base=old_base, new_base=new_base, dry_run=dry_run + ) + else: + new_head = _rewrite_cherry_pick( + record, old_base=old_base, new_base=new_base, dry_run=dry_run + ) + if not dry_run and current_branch() != original: + git("checkout", original) + + if not already_propagated: + live = _verify_live_downstream( + record, + expected_remote_head=current_head, + allowed_bases=allowed_bases, + remote=remote, + ) + expected_title = _updated_title( + live, index=record.metadata.index, total=len(chain.changesets) + ) + if not already_propagated: + push_changeset_branch( + record.branch, + remote=remote, + dry_run=dry_run, + expected_remote_head=current_head, + ) + edit_pull_request( + live.number, + remote=remote, + base=expected_base if live.base_branch != expected_base else None, + title=expected_title if live.title != expected_title else None, + dry_run=dry_run, + ) + if not dry_run: + _verify_published_downstream( + live, + expected_head=new_head, + expected_base=expected_base, + expected_title=expected_title, + remote=remote, + ) + if dry_run and not already_propagated: + new_base = record.branch + else: + new_base = new_head + + if not dry_run and current_branch() != original: + # Reaching this point means all rewrites completed without conflict. + # A conflict intentionally remains checked out for manual recovery. + if git("status", "--porcelain").stdout.strip(): + raise CommandError( + "Propagation left local conflict artifacts; resolve them before retrying." + ) + if current_branch() != original: + git("checkout", original) + + +def propagate_from_live( + *, + source: str, + base: str | None, + pr_number: int | None, + index: int | None, + strategy: str, + remote: str, + dry_run: bool, + authority_acknowledged: bool, +) -> None: + """Verify one merged PR and propagate its open downstream suffix.""" + + ensure_git_repo() + ensure_clean_tree() + _require_authority(dry_run=dry_run, authority_acknowledged=authority_acknowledged) + chain, pull_requests = _rehydrate_live(source=source, base=base, remote=remote) + record, pr = _target(chain, pull_requests, pr_number=pr_number, index=index) + target_index = record.metadata.index + _require_sequential_target(chain, target_index) + if pr.state.upper() != "MERGED": + raise CommandError( + f"PR #{pr.number} is {pr.state or 'UNKNOWN'}, not MERGED; use merge-propagate " + "to merge it under explicit authority." + ) + if not dry_run: + _verify_merged_on_base(pr, base=chain.base_branch, remote=remote) + else: + print( + f"[DRY-RUN] Would verify PR #{pr.number} is merged on {chain.base_branch}." + ) + _propagate_chain( + chain, + pull_requests, + merged_index=target_index, + strategy=strategy, + remote=remote, + dry_run=dry_run, + ) + print( + "[OK] Dry-run propagation complete." + if dry_run + else "[OK] Propagation completed." + ) + + +def merge_propagate_from_live( + *, + source: str, + base: str | None, + pr_number: int | None, + index: int | None, + strategy: str, + method: str, + remote: str, + dry_run: bool, + authority_acknowledged: bool, +) -> None: + """Merge one changeset PR, verify it remotely, then propagate downstream.""" + + ensure_git_repo() + ensure_clean_tree() + _require_authority(dry_run=dry_run, authority_acknowledged=authority_acknowledged) + chain, pull_requests = _rehydrate_live(source=source, base=base, remote=remote) + record, pr = _target(chain, pull_requests, pr_number=pr_number, index=index) + target_index = record.metadata.index + _require_sequential_target(chain, target_index) + state = pr.state.upper() + if state == "CLOSED": + raise CommandError(f"PR #{pr.number} is closed without merge.") + if state != "MERGED": + for prior in chain.changesets[: target_index - 1]: + if prior.pr_number is None or prior.pr_number not in pull_requests: + raise CommandError( + f"Preceding changeset {prior.metadata.index} has no verified PR." + ) + _verify_merged_on_base( + pull_requests[prior.pr_number], + base=chain.base_branch, + remote=remote, + ) + live_target = _verify_live_downstream( + record, + expected_remote_head=record.head, + allowed_bases={chain.base_branch}, + remote=remote, + role="Merge target", + ) + _require_merge_target_ancestry( + record, base=live_target.base_branch, remote=remote + ) + merge_pull_request( + pr.number, + expected_head=live_target.head_sha, + method=method, + remote=remote, + dry_run=dry_run, + ) + else: + print(f"[INFO] PR #{pr.number} is already merged; resuming propagation.") + if not dry_run: + _verify_merged_on_base(pr, base=chain.base_branch, remote=remote) + else: + print( + f"[DRY-RUN] Would verify PR #{pr.number} is MERGED and represented on " + f"{remote}/{chain.base_branch} before propagation." + ) + _propagate_chain( + chain, + pull_requests, + merged_index=target_index, + strategy=strategy, + remote=remote, + dry_run=dry_run, + ) + print( + "[OK] Dry-run merge-and-propagate complete." + if dry_run + else "[OK] Merge-and-propagate completed." + ) diff --git a/skills/carve-changesets/scripts/rehydrate.py b/skills/carve-changesets/scripts/rehydrate.py index 00c9b37..6597f49 100644 --- a/skills/carve-changesets/scripts/rehydrate.py +++ b/skills/carve-changesets/scripts/rehydrate.py @@ -30,6 +30,9 @@ class PullRequestRecord: base_branch: str state: str body: str + title: str = "" + merge_sha: str | None = None + is_cross_repository: bool = False @dataclass(frozen=True) @@ -70,7 +73,11 @@ def _git(cwd: Path, *args: str) -> str: def discover_changeset_heads( - cwd: Path, source_branch: str, remote: str + cwd: Path, + source_branch: str, + remote: str, + *, + prefer_remote: bool = False, ) -> dict[int, tuple[str, str]]: """Resolve current changeset refs and reject local/remote ambiguity.""" @@ -108,6 +115,9 @@ def discover_changeset_heads( local = variants.get("local") published = variants.get("remote") if local and published and local[1] != published[1]: + if prefer_remote: + heads[index] = published + continue raise RehydrationError( f"Changeset branch {local[0]} is ambiguous: local head {local[1]} " f"differs from {remote} head {published[1]}." @@ -143,18 +153,25 @@ def rehydrate_chain( base_branch: str | None = None, cwd: Path | str = Path.cwd(), remote: str = "origin", + prefer_remote: bool = False, ) -> Chain: """Reconstruct an ordered chain without consulting local plan or state files.""" if not source_branch.strip(): raise RehydrationError("Source branch must not be empty.") repo = Path(cwd) - heads = discover_changeset_heads(repo, source_branch, remote) - if not heads: + heads = discover_changeset_heads( + repo, source_branch, remote, prefer_remote=prefer_remote + ) + prs = _pr_by_branch(pull_requests, source_branch) + pr_indices = { + int(pr.head_branch.removeprefix(f"{source_branch}-")): pr for pr in prs.values() + } + found = sorted(set(heads) | set(pr_indices)) + if not found: raise RehydrationError( - f"No changeset branches named {source_branch}-N were found locally or on {remote}." + f"No changeset branches or PRs named {source_branch}-N were found." ) - found = sorted(heads) expected = list(range(1, found[-1] + 1)) if found != expected: missing = sorted(set(expected) - set(found)) @@ -164,7 +181,6 @@ def rehydrate_chain( + "." ) - prs = _pr_by_branch(pull_requests, source_branch) if base_branch is None: first_pr = prs.get(f"{source_branch}-1") if first_pr is None: @@ -178,8 +194,18 @@ def rehydrate_chain( records: list[ChangesetRecord] = [] source_sha: str | None = None slugs: set[str] = set() + prior_prs_merged = True for index in found: - branch, head = heads[index] + pr = pr_indices.get(index) + if index in heads: + branch, head = heads[index] + elif pr is not None and pr.state.upper() == "MERGED": + branch, head = pr.head_branch, pr.head_sha + _git(repo, "cat-file", "-e", f"{head}^{{commit}}") + else: + raise RehydrationError( + f"Open changeset branch {source_branch}-{index} is missing locally and on {remote}." + ) message = _git(repo, "show", "-s", "--format=%B", head) try: metadata = parse_commit_message(message) @@ -207,17 +233,26 @@ def rehydrate_chain( ) slugs.add(metadata.slug) - expected_base = base_branch if index == 1 else f"{source_branch}-{index - 1}" + predecessor_base = base_branch if index == 1 else f"{source_branch}-{index - 1}" pr = prs.get(branch) if pr is not None: + if pr.is_cross_repository: + raise RehydrationError( + f"PR #{pr.number} uses a fork head; changeset branches must belong " + "to the selected repository." + ) if pr.head_sha != head: raise RehydrationError( f"PR #{pr.number} head {pr.head_sha} disagrees with branch {branch} head {head}." ) - if pr.base_branch != expected_base: + allowed_bases = {predecessor_base} + if prior_prs_merged: + allowed_bases.add(base_branch) + if pr.base_branch not in allowed_bases: raise RehydrationError( - f"PR #{pr.number} base {pr.base_branch!r} conflicts with expected " - f"base {expected_base!r} for changeset {index}." + f"PR #{pr.number} base {pr.base_branch!r} conflicts with allowed " + f"base(s) {', '.join(repr(item) for item in sorted(allowed_bases))} " + f"for changeset {index}." ) try: pr_metadata = parse_pr_metadata(pr.body) @@ -232,11 +267,14 @@ def rehydrate_chain( metadata=metadata, branch=branch, head=head, - base=expected_base, + base=pr.base_branch if pr else predecessor_base, pr_number=pr.number if pr else None, pr_state=pr.state.upper() if pr else None, ) ) + prior_prs_merged = ( + prior_prs_merged and pr is not None and pr.state.upper() == "MERGED" + ) assert source_sha is not None return Chain( diff --git a/skills/carve-changesets/scripts/tests/test_cli_safety.py b/skills/carve-changesets/scripts/tests/test_cli_safety.py index cdc6e89..f3bc2ba 100644 --- a/skills/carve-changesets/scripts/tests/test_cli_safety.py +++ b/skills/carve-changesets/scripts/tests/test_cli_safety.py @@ -11,14 +11,19 @@ class CliSafetyTests(unittest.TestCase): def test_every_operation_has_one_mutation_class(self) -> None: parser = build_parser() help_text = parser.format_help() - self.assertEqual(14, len(COMMAND_MUTATION_CLASSES)) + self.assertEqual(16, len(COMMAND_MUTATION_CLASSES)) for command, mutation_class in COMMAND_MUTATION_CLASSES.items(): self.assertIn(command, help_text) self.assertIn(f"[{mutation_class}]", help_text) def test_all_remote_mutations_default_to_dry_run(self) -> None: parser = build_parser() - for argv in (("pr-create",), ("push-chain",)): + for argv in ( + ("pr-create",), + ("push-chain",), + ("propagate", "--source", "feature/test", "--index", "1"), + ("merge-propagate", "--source", "feature/test", "--index", "1"), + ): args = parser.parse_args(argv) self.assertEqual("remote-mutating", args.mutation_class) self.assertTrue(args.dry_run) diff --git a/skills/carve-changesets/scripts/tests/test_github.py b/skills/carve-changesets/scripts/tests/test_github.py index 632fd43..165f142 100644 --- a/skills/carve-changesets/scripts/tests/test_github.py +++ b/skills/carve-changesets/scripts/tests/test_github.py @@ -12,6 +12,80 @@ class GithubTests(unittest.TestCase): + def test_shared_pr_decoder_reports_operation_context(self) -> None: + with self.assertRaisesRegex( + CommandError, "changeset PR for feature/test-2.*valid PR number" + ): + github_mod._pull_request_record( + {"number": "not-a-number"}, + context="changeset PR for feature/test-2", + ) + + def test_pr_merge_fences_the_exact_number_and_head(self) -> None: + with ( + mock.patch.object( + github_mod, + "github_repo_for_remote", + return_value="github.com/acme/widgets", + ), + mock.patch.object(github_mod, "ensure_gh_ready"), + mock.patch.object(github_mod, "gh_capture") as capture, + ): + github_mod.merge_pull_request( + 94, + expected_head="a" * 40, + method="squash", + remote="origin", + dry_run=False, + ) + + self.assertEqual( + ( + "pr", + "merge", + "94", + "-R", + "github.com/acme/widgets", + "--squash", + "--match-head-commit", + "a" * 40, + ), + capture.call_args.args[0], + ) + + def test_pr_edit_targets_explicit_number(self) -> None: + with ( + mock.patch.object( + github_mod, + "github_repo_for_remote", + return_value="github.com/acme/widgets", + ), + mock.patch.object(github_mod, "ensure_gh_ready"), + mock.patch.object(github_mod, "gh_capture") as capture, + ): + github_mod.edit_pull_request( + 93, + remote="origin", + base="main", + title="Feature (2 of 3)", + dry_run=False, + ) + + self.assertEqual( + ( + "pr", + "edit", + "93", + "-R", + "github.com/acme/widgets", + "--base", + "main", + "--title", + "Feature (2 of 3)", + ), + capture.call_args.args[0], + ) + def test_pr_create_dry_run_uses_body_file(self) -> None: repo_dir, plan = init_repo() try: diff --git a/skills/carve-changesets/scripts/tests/test_propagate.py b/skills/carve-changesets/scripts/tests/test_propagate.py index 0009050..1b5b681 100644 --- a/skills/carve-changesets/scripts/tests/test_propagate.py +++ b/skills/carve-changesets/scripts/tests/test_propagate.py @@ -1,15 +1,44 @@ from __future__ import annotations import shutil +import tempfile import unittest +from argparse import Namespace +from pathlib import Path from unittest import mock +import helpers +import propagate as propagate_mod from chain import create_chain +from cli import cmd_merge_propagate +from common import CommandError from legacy_helpers import chdir, init_remote, init_repo, run -from propagate import push_chain +from metadata import ChangesetMetadata, embed_pr_metadata, stamp_commit_message +from propagate import ( + merge_propagate_from_live, + propagate_from_live, + push_chain, + push_changeset_branch, +) +from rehydrate import PullRequestRecord class PushChainTests(unittest.TestCase): + def test_propagation_push_rejects_remote_head_moved_since_rehydration(self) -> None: + with ( + mock.patch("propagate.remote_branch_head", return_value="b" * 40), + mock.patch("propagate.git") as git_call, + ): + with self.assertRaisesRegex(CommandError, "moved from verified head"): + push_changeset_branch( + "feature/test-2", + remote="origin", + dry_run=False, + expected_remote_head="a" * 40, + ) + + git_call.assert_not_called() + def test_push_chain_never_sends_base_or_source_to_force_push(self) -> None: repo_dir, plan = init_repo() remote_dir = None @@ -32,6 +61,509 @@ def test_push_chain_never_sends_base_or_source_to_force_push(self) -> None: if remote_dir is not None: shutil.rmtree(remote_dir.parent) + +class StatelessPropagationTests(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = Path(tempfile.mkdtemp()) + self.repo, self.bare, _ = helpers.init_repo(self.temp_dir) + helpers.run( + self.temp_dir, + "git", + "--git-dir", + str(self.bare), + "symbolic-ref", + "HEAD", + "refs/heads/main", + ) + helpers.run(self.repo, "git", "checkout", "feature/report") + (self.repo / "second.txt").write_text("second source part\n") + (self.repo / "third.txt").write_text("third source part\n") + helpers.run(self.repo, "git", "add", "second.txt", "third.txt") + self.source_sha = helpers.commit(self.repo, "complete source") + helpers.run(self.repo, "git", "push", "--force", "origin", "feature/report") + + self.prs: dict[int, PullRequestRecord] = {} + previous = "main" + for index, filename in ( + (1, "source.txt"), + (2, "second.txt"), + (3, "third.txt"), + ): + branch = f"feature/report-{index}" + helpers.run(self.repo, "git", "checkout", "-b", branch, previous) + content = helpers.run( + self.repo, "git", "show", f"feature/report:{filename}" + ) + (self.repo / filename).write_text(content + "\n") + helpers.run(self.repo, "git", "add", filename) + metadata = ChangesetMetadata( + slug=f"part-{index}", + index=index, + source_branch="feature/report", + source_sha=self.source_sha, + ) + head = helpers.commit( + self.repo, + stamp_commit_message(f"feat: changeset {index}", metadata), + ) + helpers.run(self.repo, "git", "push", "-u", "origin", branch) + self.prs[100 + index] = PullRequestRecord( + number=100 + index, + head_branch=branch, + head_sha=head, + base_branch=("main" if index == 1 else f"feature/report-{index - 1}"), + state="OPEN", + body=embed_pr_metadata("## Overall Feature\n\nReport API\n", metadata), + title=f"Report API ({index} of 1)", + ) + previous = branch + + state = self.repo / ".carve-changesets" + state.mkdir() + (state / "plan.json").write_text("{}\n") + shutil.rmtree(state) + helpers.run(self.repo, "git", "checkout", "feature/report") + + def tearDown(self) -> None: + shutil.rmtree(self.temp_dir) + + def _live_pr(self, number: int, **_kwargs) -> PullRequestRecord: + pr = self.prs[number] + remote_output = helpers.run( + self.repo, + "git", + "ls-remote", + "origin", + f"refs/heads/{pr.head_branch}", + ) + remote_head = remote_output.split()[0] if remote_output else pr.head_sha + return PullRequestRecord(**{**pr.__dict__, "head_sha": remote_head}) + + def _merge(self, number: int, **_kwargs) -> None: + self.assertEqual(101, number) + original = helpers.run(self.repo, "git", "branch", "--show-current") + helpers.run(self.repo, "git", "checkout", "main") + helpers.run( + self.repo, "git", "merge", "--no-ff", "--no-edit", "feature/report-1" + ) + merge_sha = helpers.run(self.repo, "git", "rev-parse", "HEAD") + helpers.run(self.repo, "git", "push", "origin", "main") + helpers.run(self.repo, "git", "checkout", original) + self.prs[number] = PullRequestRecord( + **{ + **self.prs[number].__dict__, + "state": "MERGED", + "merge_sha": merge_sha, + } + ) + + def _edit(self, number: int, *, base=None, title=None, **_kwargs) -> None: + pr = self.prs[number] + self.prs[number] = PullRequestRecord( + **{ + **pr.__dict__, + "base_branch": base or pr.base_branch, + "title": title or pr.title, + } + ) + + def _all_live_prs(self) -> list[PullRequestRecord]: + return [self._live_pr(number) for number in sorted(self.prs)] + + def _fresh_clone(self, name: str) -> Path: + clone = self.temp_dir / name + helpers.run(self.temp_dir, "git", "clone", str(self.bare), str(clone)) + helpers.run(clone, "git", "config", "user.name", "Carve Tests") + helpers.run(clone, "git", "config", "user.email", "carve@example.test") + return clone + + def _run_combined(self, strategy: str, *, through_cli: bool = False) -> None: + real_push = propagate_mod.push_changeset_branch + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: [ + self._live_pr(101), + self._live_pr(102), + self._live_pr(103), + ], + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object( + propagate_mod, "merge_pull_request", side_effect=self._merge + ), + mock.patch.object( + propagate_mod, "edit_pull_request", side_effect=self._edit + ) as edit, + mock.patch.object( + propagate_mod, "push_changeset_branch", wraps=real_push + ) as push, + ): + if through_cli: + cmd_merge_propagate( + Namespace( + source="feature/report", + base="main", + pr=101, + index=None, + strategy=strategy, + method="merge", + remote="origin", + dry_run=False, + ack_merge_and_propagate=True, + ) + ) + else: + merge_propagate_from_live( + source="feature/report", + base="main", + pr_number=None, + index=1, + strategy=strategy, + method="merge", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertFalse((self.repo / ".carve-changesets").exists()) + main = helpers.run(self.repo, "git", "ls-remote", "origin", "refs/heads/main") + branch_one = helpers.run( + self.repo, + "git", + "ls-remote", + "origin", + "refs/heads/feature/report-1", + ) + branch_two = helpers.run( + self.repo, + "git", + "ls-remote", + "origin", + "refs/heads/feature/report-2", + ) + branch_three = helpers.run( + self.repo, + "git", + "ls-remote", + "origin", + "refs/heads/feature/report-3", + ) + self.assertEqual(self.prs[101].merge_sha, main.split()[0]) + self.assertEqual(self.prs[101].head_sha, branch_one.split()[0]) + self.assertEqual(self._live_pr(102).head_sha, branch_two.split()[0]) + self.assertEqual(self._live_pr(103).head_sha, branch_three.split()[0]) + self.assertTrue( + helpers.run( + self.repo, + "git", + "merge-base", + "--is-ancestor", + main.split()[0], + branch_two.split()[0], + ) + == "" + ) + self.assertEqual("main", self.prs[102].base_branch) + self.assertEqual("feature/report-2", self.prs[103].base_branch) + self.assertEqual("Report API (2 of 3)", self.prs[102].title) + self.assertEqual("Report API (3 of 3)", self.prs[103].title) + self.assertEqual([102, 103], [call.args[0] for call in edit.call_args_list]) + self.assertEqual( + ["feature/report-2", "feature/report-3"], + [call.args[0] for call in push.call_args_list], + ) + + def test_cmd_merge_propagate_rehydrates_without_state_and_rebases(self) -> None: + self._run_combined("rebase", through_cli=True) + + def test_merge_propagate_supports_cherry_pick(self) -> None: + self._run_combined("cherry-pick") + + def test_propagate_resumes_after_remote_merge_without_local_state(self) -> None: + self._merge(101) + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: [ + self._live_pr(101), + self._live_pr(102), + self._live_pr(103), + ], + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object( + propagate_mod, "edit_pull_request", side_effect=self._edit + ), + ): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertEqual("main", self.prs[102].base_branch) + self.assertEqual("Report API (2 of 3)", self.prs[102].title) + self.assertEqual("Report API (3 of 3)", self.prs[103].title) + + def test_execution_requires_authority_acknowledgement(self) -> None: + with chdir(self.repo): + with self.assertRaisesRegex(CommandError, "ack-merge-and-propagate"): + merge_propagate_from_live( + source="feature/report", + base="main", + pr_number=None, + index=1, + strategy="rebase", + method="merge", + remote="origin", + dry_run=False, + authority_acknowledged=False, + ) + + def test_fresh_clone_resumes_after_merged_head_branch_is_deleted(self) -> None: + self._merge(101) + helpers.run( + self.repo, + "git", + "push", + "origin", + "--delete", + "feature/report-1", + ) + clone = self._fresh_clone("deleted-merged-head") + + with ( + chdir(clone), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object( + propagate_mod, "edit_pull_request", side_effect=self._edit + ), + ): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertEqual("main", self.prs[102].base_branch) + self.assertEqual("Report API (3 of 3)", self.prs[103].title) + + def test_partial_propagation_resumes_only_missing_remote_work(self) -> None: + self._merge(101) + old_second = self._live_pr(102).head_sha + old_third = self._live_pr(103).head_sha + + def fail_first_edit(number: int, **_kwargs) -> None: + if number == 102: + raise CommandError("injected edit failure") + self._edit(number, **_kwargs) + + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object( + propagate_mod, "edit_pull_request", side_effect=fail_first_edit + ), + ): + with self.assertRaisesRegex(CommandError, "injected edit failure"): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertNotEqual(old_second, self._live_pr(102).head_sha) + self.assertEqual(old_third, self._live_pr(103).head_sha) + clone = self._fresh_clone("partial-frontier") + real_push = propagate_mod.push_changeset_branch + with ( + chdir(clone), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object( + propagate_mod, "edit_pull_request", side_effect=self._edit + ), + mock.patch.object( + propagate_mod, "push_changeset_branch", wraps=real_push + ) as push, + ): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertEqual( + ["feature/report-3"], [call.args[0] for call in push.call_args_list] + ) + self.assertEqual("main", self.prs[102].base_branch) + self.assertEqual("Report API (3 of 3)", self.prs[103].title) + + def test_pr_state_change_after_planning_withholds_force_push(self) -> None: + self._merge(101) + old_second = self._live_pr(102).head_sha + + def close_second(number: int, **_kwargs) -> PullRequestRecord: + live = self._live_pr(number) + if number == 102: + return PullRequestRecord(**{**live.__dict__, "state": "CLOSED"}) + return live + + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=close_second + ), + ): + with self.assertRaisesRegex(CommandError, "changed to CLOSED"): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + self.assertEqual(old_second, self._live_pr(102).head_sha) + + def test_merge_target_still_based_on_merged_predecessor_is_withheld( + self, + ) -> None: + self._merge(101) + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=self._live_pr + ), + mock.patch.object(propagate_mod, "merge_pull_request") as merge, + ): + with self.assertRaisesRegex( + CommandError, "Resume propagation for the preceding merged changeset" + ): + merge_propagate_from_live( + source="feature/report", + base="main", + pr_number=None, + index=2, + strategy="rebase", + method="merge", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + merge.assert_not_called() + self.assertEqual("OPEN", self.prs[102].state) + self.assertEqual("feature/report-1", self.prs[102].base_branch) + + def test_concurrent_unrelated_retarget_withholds_push_and_edit(self) -> None: + self._merge(101) + local_second = helpers.run( + self.repo, "git", "rev-parse", "refs/heads/feature/report-2" + ) + + def retarget_second(number: int, **_kwargs) -> PullRequestRecord: + live = self._live_pr(number) + if number == 102: + return PullRequestRecord( + **{**live.__dict__, "base_branch": "release/unrelated"} + ) + return live + + with ( + chdir(self.repo), + mock.patch.object( + propagate_mod, + "pull_requests_for_source", + side_effect=lambda *_args, **_kwargs: self._all_live_prs(), + ), + mock.patch.object( + propagate_mod, "pull_request_by_number", side_effect=retarget_second + ), + mock.patch.object(propagate_mod, "push_changeset_branch") as push, + mock.patch.object(propagate_mod, "edit_pull_request") as edit, + ): + with self.assertRaisesRegex(CommandError, "release/unrelated"): + propagate_from_live( + source="feature/report", + base="main", + pr_number=101, + index=None, + strategy="rebase", + remote="origin", + dry_run=False, + authority_acknowledged=True, + ) + + push.assert_not_called() + edit.assert_not_called() + self.assertEqual( + local_second, + helpers.run(self.repo, "git", "rev-parse", "refs/heads/feature/report-2"), + ) + def test_push_chain_uses_exact_refspecs_and_leases(self) -> None: repo_dir, plan = init_repo() remote_dir = None diff --git a/skills/carve-changesets/scripts/tests/test_rehydrate.py b/skills/carve-changesets/scripts/tests/test_rehydrate.py index f0783dc..69bff8f 100644 --- a/skills/carve-changesets/scripts/tests/test_rehydrate.py +++ b/skills/carve-changesets/scripts/tests/test_rehydrate.py @@ -145,11 +145,11 @@ def test_conflicting_pr_base_fails_closed(self) -> None: _, prs = self._materialize() clone = self._fresh_clone() conflicting = [ - prs[0], + PullRequestRecord(**{**prs[0].__dict__, "state": "OPEN"}), PullRequestRecord(**{**prs[1].__dict__, "base_branch": "main"}), ] - with self.assertRaisesRegex(RehydrationError, "conflicts with expected base"): + with self.assertRaisesRegex(RehydrationError, "conflicts with allowed base"): rehydrate_chain( source_branch="feature/report", pull_requests=conflicting, cwd=clone ) @@ -170,6 +170,19 @@ def test_trailer_and_pr_metadata_disagreement_fails_closed(self) -> None: source_branch="feature/report", pull_requests=conflicting, cwd=clone ) + def test_cross_repository_changeset_pr_fails_closed(self) -> None: + _, prs = self._materialize() + clone = self._fresh_clone() + forked = [ + PullRequestRecord(**{**prs[0].__dict__, "is_cross_repository": True}), + prs[1], + ] + + with self.assertRaisesRegex(RehydrationError, "uses a fork head"): + rehydrate_chain( + source_branch="feature/report", pull_requests=forked, cwd=clone + ) + if __name__ == "__main__": unittest.main() diff --git a/skills/carve-changesets/scripts/validate.py b/skills/carve-changesets/scripts/validate.py index 8621ce0..180ca9e 100644 --- a/skills/carve-changesets/scripts/validate.py +++ b/skills/carve-changesets/scripts/validate.py @@ -85,6 +85,7 @@ def validate_live_chain( *, cwd: Path | str = Path.cwd(), remote: str = "origin", + allow_partial_propagation: bool = False, ) -> ChainValidation: """Check ancestry, source identity, and equivalence using only live git.""" @@ -139,15 +140,23 @@ def validate_live_chain( ) expected_indices = {item.metadata.index for item in chain.changesets} - if live_heads is not None and set(live_heads) != expected_indices: - diagnostics.append( - ValidationDiagnostic( - "chain_shape_changed", - "error", - "Current changeset branch indices differ from the rehydrated chain: " - f"expected {sorted(expected_indices)}, found {sorted(live_heads)}.", + if live_heads is not None: + missing_open = { + item.metadata.index + for item in chain.changesets + if item.pr_state != "MERGED" and item.metadata.index not in live_heads + } + unexpected = set(live_heads) - expected_indices + if missing_open or unexpected: + diagnostics.append( + ValidationDiagnostic( + "chain_shape_changed", + "error", + "Current open changeset branch indices differ from the rehydrated " + f"chain: missing open {sorted(missing_open)}, unexpected " + f"{sorted(unexpected)}.", + ) ) - ) base_head = _resolve_branch(repo, chain.base_branch, remote) if base_head is None: @@ -159,13 +168,15 @@ def validate_live_chain( ) ) - predecessor = base_head - predecessor_name = chain.base_branch + open_changeset_seen = False + merged_changeset_seen = False + rehydrated_heads = {item.branch: item.head for item in chain.changesets} for changeset in chain.changesets: live = ( live_heads.get(changeset.metadata.index) if live_heads is not None else None ) - if live is None: + is_merged = changeset.pr_state == "MERGED" + if live is None and not is_merged: diagnostics.append( ValidationDiagnostic( "changeset_ref_missing", @@ -174,6 +185,8 @@ def validate_live_chain( ) ) head = None + elif live is None: + head = changeset.head else: live_branch, head = live if live_branch != changeset.branch or head != changeset.head: @@ -186,9 +199,50 @@ def validate_live_chain( ) ) - if head is not None and predecessor is not None: + if is_merged and open_changeset_seen: + diagnostics.append( + ValidationDiagnostic( + "merge_sequence_broken", + "error", + f"Changeset branch {changeset.branch} is merged after an unmerged " + "changeset; merges must remain a leading sequence.", + ) + ) + if not is_merged: + open_changeset_seen = True + else: + merged_changeset_seen = True + + predecessor_name = changeset.base + predecessor = _resolve_branch(repo, predecessor_name, remote) + if predecessor is None: + predecessor = rehydrated_heads.get(predecessor_name) + if not is_merged and predecessor is None: + diagnostics.append( + ValidationDiagnostic( + "predecessor_missing", + "error", + f"Predecessor {predecessor_name!r} for {changeset.branch} is not " + "available in live git.", + ) + ) + if not is_merged and head is not None and predecessor is not None: ancestry = _is_ancestor(repo, predecessor, head) - if ancestry is False: + if ( + ancestry is False + and allow_partial_propagation + and merged_changeset_seen + ): + diagnostics.append( + ValidationDiagnostic( + "partial_propagation_frontier", + "warning", + f"Changeset branch {changeset.branch} is still based on its " + "pre-propagation predecessor; propagation must validate and " + "advance this live frontier.", + ) + ) + elif ancestry is False: diagnostics.append( ValidationDiagnostic( "predecessor_ancestry_broken", @@ -206,13 +260,13 @@ def validate_live_chain( f"{predecessor} with {changeset.branch} at {head}.", ) ) - predecessor = head - predecessor_name = changeset.branch if stamped_source is not None and chain.changesets and live_heads is not None: tip_record = chain.changesets[-1] live_tip = live_heads.get(tip_record.metadata.index) tip = live_tip[1] if live_tip is not None else None + if tip is None and tip_record.pr_state == "MERGED": + tip = base_head source_tree = _resolve(repo, f"{stamped_source}^{{tree}}") tip_tree = _resolve(repo, f"{tip}^{{tree}}") if tip is not None else None if source_tree is None or tip_tree is None: