From b6b074f97b6032c1c2df41fc2f75261f36394ebc Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 27 Jul 2026 23:42:25 -0700 Subject: [PATCH 1/2] fix(release): harden partial publish recovery and validation --- CONTRIBUTING.md | 84 ------ DEVELOPMENT.md | 139 +++++----- Makefile | 4 +- scripts/release.py | 26 +- scripts/release/publish.py | 40 +-- scripts/release/smoke.py | 68 ++++- scripts/release/state_checks.py | 23 +- scripts/release/state_state.py | 53 +++- scripts/release/state_types.py | 54 +++- tests/unit/packaging/test_release_surfaces.py | 82 ++---- .../packaging/test_release_tool_publish.py | 220 ++++++++++++++- .../unit/packaging/test_release_tool_smoke.py | 67 +++++ .../unit/packaging/test_release_tool_state.py | 82 ++++++ .../test_release_tool_state_checks.py | 252 ++++++++++++++++++ 14 files changed, 922 insertions(+), 272 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69e7f9b..a3fbd07 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,87 +50,3 @@ uvx pre-commit==4.6.0 run --all-files --show-diff-on-failure [the UI compact dashboard](docs/architecture/ui_compact_dashboard.md) for the intended graph behavior and add or update the relevant layout fixture under [tests/unit/observability/fixtures/](tests/unit/observability/fixtures/). - -## Release Surface Checks - -After manually updating `pyproject.toml` and `CHANGELOG.md`, run: - -```bash -make release-prepare -make release-check -``` - -Optional tools such as `pipx`, `npm`, and Homebrew may skip locally when absent, -but static release-surface tests should still pass. - -To publish PyPI and npm after the checks pass and registry credentials are -configured: - -```bash -make release -``` - -`make release-prepare` synchronizes generated release metadata from -`pyproject.toml`, refreshes `uv.lock`, builds local PyPI and npm artifacts plus -the offline runtime wheelhouse, writes release manifests, and prepares the -Homebrew formula candidate. It fails if the target version already exists on -PyPI or npm. - -The exact Hatchling build-system pin keeps the manual release and its immediately -following GitHub Release rebuild on the same backend version. The manually -dispatched `release-artifacts` command rebuilds the registry artifacts needed for -verification but skips the offline wheelhouse, which stays local to release -preparation and explicit wheelhouse checks. - -`make release-check` is state-aware. For unpublished versions it verifies -generated metadata and runs lint, format-check, tests, package checks, and -install smokes. For already completed releases it verifies PyPI, npm, npm -`latest`, Homebrew formula metadata, and the Git tag, then exits successfully -without rerunning pre-publish smokes. It prints a final reminder to verify the -changelog because changelog content is still reviewed manually. - -`make release` asks for exact version confirmation, reruns `make release-check`, -publishes PyPI first, publishes npm with the `latest` dist-tag, reconciles npm -`latest`, and creates/pushes the annotated Git tag after both registries verify. -If a release is interrupted after only one registry is updated, fix the issue and -run `make release-pypi` or `make release-npm` to complete the missing side. In -that registry-recovery path, `make release` and the publish targets allow local -worktree and branch state to differ while still verifying generated metadata, -release-manifest artifacts, registry artifacts, and tag conflicts. In -non-TTY npm two-factor flows, use `NPM_PUBLISH_OTP` and `NPM_DIST_TAG_OTP` so -`npm publish` and `npm dist-tag add` each receive a fresh OTP. Homebrew tap -publishing is still a separate maintainer step: copy the prepared formula into -the tap, run audit/test there, and push the tap update. - -Immediately after the manual `make release` flow creates and pushes the tag, -dispatch `.github/workflows/release.yml` from `master` and provide that tag as -the required `tag` input, before `master` advances. The workflow rejects any -other dispatch ref. At the start of verification, it checks out the requested -tag, resolves its commit, and requires it to match the master commit that -dispatched the workflow exactly. Historical-tag backfills and new dispatches -after `master` advances are unsupported. It rebuilds from that tagged source -with `scripts/release.py release-artifacts`, verifies PyPI, npm, Homebrew, -manifest, local artifact, and tag state with -`scripts/release.py github-release-plan`, and transfers the verified artifacts -and manifest to the write-authorized job. The publishing job checks out the -exact verified commit, verifies fresh external state before mutating GitHub, -repeats the plan, and supplies the verified predecessor tag explicitly when -GitHub generates release notes. Every plan evaluation re-fetches -`origin/master` and requires the release commit to remain reachable from it. The -publisher then reloads and re-verifies draft assets immediately before -publication. The GitHub Release contains `dist/*`; the npm tarball is an -integrity input, not a GitHub Release asset. It verifies exact asset names, -sizes, and GitHub SHA-256 digests before and after publication, rejects -unexpected draft assets, and never mutates an already-published mismatch. The -release title must match the tag, and generated notes carry a hidden, versioned -automation marker; existing drafts without both are rejected before asset -mutation. -Prereleases can never be GitHub `Latest`. A stable release is `Latest` only when -it is the highest published stable version on PyPI. A repository-wide -non-dropping queue serializes publication workflows around those checks and -mutations. PyPI and npm remain the production publishing sources of truth; the -workflow does not publish production packages. -`.github/workflows/testpypi.yml` is the separate TestPyPI Trusted Publishing -workflow. Maintainers may intentionally dispatch it from any selected ref; it is -not restricted to `master`. It fails if the package version already exists on -TestPyPI. diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index aa7864f..925a773 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -68,65 +68,9 @@ Current CI policy: - Default branch: `master`. - The supported platform matrix is defined in [Supported Platforms](#supported-platforms). -- Production publishing is local-only. Maintainers run `make release` to - publish PyPI, publish npm with the `latest` dist-tag, and create/push the - final Git tag. GitHub Actions does not publish production PyPI or npm - packages and does not need PyPI or npm credentials. -- `.github/workflows/release.yml` provides manually dispatched GitHub Release - automation as the second phase of the same release operation: - - 1. **Create the production release locally.** The manual `make release` flow - is the production source of truth. - 2. **Dispatch GitHub publication from the same commit.** Immediately after - `make release` creates and pushes the final tag, dispatch the workflow from - `master` with the required `tag` input, before `master` advances. The - workflow rejects dispatches outside `refs/heads/master`. It checks out the - requested tag, resolves its commit, and requires it to match the master - commit that dispatched the workflow exactly. Historical-tag backfills and - new dispatches after `master` advances are unsupported. - 3. **Verify the tagged source.** The workflow rebuilds the artifacts and - verifies their manifest, registry state, Homebrew formula metadata, and tag - state through `scripts/release.py`. It transfers the verified sdist, wheel, - npm tarball, and manifest to the write-authorized job. - 4. **Publish the GitHub Release.** The publisher checks out the exact verified - commit and verifies fresh registry and tag state before mutating GitHub. It - repeats the plan after draft-asset verification immediately before - publication and supplies the verified predecessor tag when GitHub generates - release notes. The GitHub Release itself contains only `dist/*`. - -- GitHub Release publication fails closed: - - - Every plan evaluation re-fetches `origin/master` and requires the release - commit to remain reachable from it. - - Draft and published assets are checked by exact name, size, and GitHub's - immutable upload-time SHA-256 digest. API failures, truncated responses, and - unexpected assets fail closed. - - Release titles must exactly match their tags, and generated notes carry a - hidden, versioned automation marker. Existing drafts without both are - rejected before any asset mutation. - - New releases are built as drafts, verified, published, and verified again. - Prereleases can never be GitHub `Latest`; a stable release is `Latest` only - when it is the highest published stable version on PyPI. - - A repository-wide non-dropping concurrency queue serializes publication - workflows around those checks and mutations. PyPI and npm remain the - production publishing sources of truth. - -- Release artifact builds are reproducible: - - - `pyproject.toml` uses an exact Hatchling build-system pin so the manual - release and its immediately following GitHub Release rebuild use the same - backend version. - - The manually dispatched job rebuilds only the registry artifacts needed for - verification. `make release-prepare` owns the offline runtime wheelhouse used - by maintainer checks. -- The release workflow verifies the Homebrew formula metadata in this repo, but - does not update the Homebrew tap; tap publishing remains a separate maintainer - step. -- `.github/workflows/testpypi.yml` remains the separate TestPyPI Trusted - Publishing workflow. Maintainers may intentionally dispatch it from any - selected ref; it is not restricted to `master`. It fails if the package - version already exists on TestPyPI. Trusted Publishing is free and uses - short-lived OIDC credentials instead of a stored package token. +- Production publishing is local-only. Follow the + [Release Workflow](#release-workflow). GitHub Actions does not publish + production PyPI or npm packages and does not need their credentials. - Workflow actions are pinned to immutable commits, with version comments kept beside each pin for dependency automation and review. Workflow `uv` installations are also version-pinned. @@ -199,10 +143,81 @@ See [ADR 0013](docs/architecture/adr/0013-version-source-of-truth-and-documentat During the public-alpha `0.x` period, support the current schema only. Persisted run artifacts are disposable audit outputs, not migration targets; stale preflight plans may be rejected by explicit shape validation even when they carry the current `SCHEMA_VERSION`. -## Release Workflows +## Release Workflow -Use `make help` for target details and [CONTRIBUTING.md](CONTRIBUTING.md) for -the maintainer release flow. +Production releases have two publication phases: publish the packages and Git +tag locally, then publish the GitHub Release. Use `make help` for target +details. + +### 1. Prepare and validate + +Update the version in `pyproject.toml` and add the matching section to +`CHANGELOG.md`. Review the changelog content manually, then run: + +```bash +make release-prepare +make release-check +``` + +Both commands must pass before publication. Preparation stops if the target +version already exists on PyPI or npm. Some install checks may be skipped when +optional local tools such as `pipx`, npm, or Homebrew are unavailable; review +the skip messages before continuing. + +### 2. Publish packages and the Git tag + +Configure the PyPI and npm credentials, then run: + +```bash +make release +``` + +Confirm the exact version when prompted. A successful run publishes PyPI, +publishes npm with the `latest` dist-tag, and pushes the annotated Git tag. + +For a non-interactive npm release that requires two-factor authentication, set +`NPM_PUBLISH_OTP` or `NPM_OTP`. A separate npm `latest` recovery uses +`NPM_DIST_TAG_OTP` or `NPM_OTP`. + +### 3. Publish the GitHub Release + +Immediately after `make release` pushes the tag, dispatch +`.github/workflows/release.yml` from `master`. Use the full Git tag, including +the `v` prefix, as the required `tag` input: for package version `0.1.4`, enter +`v0.1.4`, not `0.1.4`. Do this before `master` advances: dispatching from +another ref, backfilling a historical tag, or dispatching after a newer commit +reaches `master` is unsupported. + +The workflow verifies the tagged release and publishes the GitHub Release. It +does not publish the production PyPI or npm packages. Prereleases are never +marked as GitHub `Latest`; a stable release is marked `Latest` only when it is +the highest published stable version on PyPI. + +### 4. Publish Homebrew separately + +Copy the prepared formula into the Homebrew tap, run the tap's audit and test +steps, and push the tap update. + +### Recover an interrupted release + +If only part of PyPI or one registry was published, fix the reported problem +and rerun the corresponding target: + +```bash +make release-pypi +make release-npm +``` + +These targets verify anything already published and complete only the missing +work. Use `make release-npm` when the npm package exists but its `latest` +dist-tag is stale. Once both registries are complete, rerun `make release` to +finish the Git tag. + +### TestPyPI + +Use `.github/workflows/testpypi.yml` for TestPyPI Trusted Publishing. It may be +dispatched from any selected ref and stops if that package version already +exists on TestPyPI. ## Key Modules diff --git a/Makefile b/Makefile index 2f70f13..73ddc8d 100644 --- a/Makefile +++ b/Makefile @@ -57,11 +57,11 @@ help: ' package-build Build wheel and sdist into dist/' \ ' package-check Build artifacts, run twine check, verify Homebrew sdist SHA' \ ' install-smoke Exercise pip, uv tool, and pipx installs where available' \ - ' install-check Run package checks plus installer, npm, and Homebrew smokes' \ + ' install-check Build once, then run package, installer, npm, and Homebrew smokes' \ '' \ 'Normal release flow:' \ ' release-prepare Sync generated metadata and prepare local release artifacts' \ - ' release-check Run pre-publish checks, or verify a completed release no-op' \ + ' release-check Verify changelog and pre-publish checks, or a completed release' \ ' release Confirm, check, publish PyPI, publish npm latest, and tag' \ '' \ 'Partial completion:' \ diff --git a/scripts/release.py b/scripts/release.py index ac63342..59b6df7 100644 --- a/scripts/release.py +++ b/scripts/release.py @@ -1,10 +1,12 @@ from __future__ import annotations import argparse +import re import subprocess import sys from pathlib import Path +from markdown_it import MarkdownIt from release import build, publish, smoke from release.state import ( CommandRunner, @@ -161,6 +163,7 @@ def release_check(root: Path, runner: CommandRunner) -> int: and npm.latest == context.version.npm and not formula_issues and is_tag_only_missing_error(tag_issues) + and is_tag_only_missing_error(list(state.reasons)) ): print( "Release registries are published and Git tag is missing; pre-publish checks are not needed." @@ -169,10 +172,8 @@ def release_check(root: Path, runner: CommandRunner) -> int: if state.status in {ReleaseStatus.PARTIAL, ReleaseStatus.BLOCKED}: return 1 fail_if_generated_metadata_stale(context, manifest) + changelog_check(root) run_pre_publish_checks(root, runner) - print( - "Verify CHANGELOG.md describes the pyproject.toml version before running make release." - ) return 0 @@ -186,10 +187,21 @@ def run_pre_publish_checks(root: Path, runner: CommandRunner) -> None: def changelog_check(root: Path) -> None: context = read_release_context(root) text = (root / "CHANGELOG.md").read_text(encoding="utf-8") - if ( - f"## [{context.version.project}]" not in text - and f"## {context.version.project}" not in text - ): + version = re.escape(context.version.project) + version_heading = re.compile(rf"^(?:\[{version}\]|{version})(?:[ \t]+.*)?$") + tokens = MarkdownIt("commonmark").parse(text) + has_version_heading = any( + opening.type == "heading_open" + and opening.tag == "h2" + and opening.level == 0 + and opening.markup == "##" + and inline.type == "inline" + and version_heading.fullmatch( + "".join(child.content for child in inline.children or ()) + ) + for opening, inline in zip(tokens, tokens[1:], strict=False) + ) + if not has_version_heading: raise ReleaseError( f"CHANGELOG.md does not contain a section for {context.version.project}. " "Changelog content is still reviewed manually." diff --git a/scripts/release/publish.py b/scripts/release/publish.py index 73b01ef..b56728c 100644 --- a/scripts/release/publish.py +++ b/scripts/release/publish.py @@ -21,6 +21,7 @@ ReleaseError, ReleaseManifest, ReleaseStatus, + RetryableRegistryError, command_exists, derive_release_state, fail_if_generated_metadata_stale, @@ -28,6 +29,7 @@ inspect_release_tag_state, is_tag_only_missing_error, manifest_context_issues, + missing_pypi_artifact_keys, print_state, query_npm_release, query_pypi_release, @@ -40,6 +42,7 @@ verify_git_tag_state, verify_local_manifest_artifacts, verify_npm_artifact, + verify_present_pypi_artifacts, verify_pypi_artifacts, ) @@ -81,7 +84,6 @@ def publish_pypi(root: Path, runner: CommandRunner, execute: bool) -> int: return 1 manifest = read_manifest(root) fail_if_generated_metadata_stale(context, manifest) - require_pypi_auth() pypi = query_pypi_release(context) npm = query_npm_release(context) require_publish_git_state( @@ -90,21 +92,19 @@ def publish_pypi(root: Path, runner: CommandRunner, execute: bool) -> int: pypi.exists, is_registry_recovery(pypi, npm), ) - issues = verify_existing_pypi(context, pypi, manifest) + issues = verify_present_pypi_artifacts(context, pypi, manifest) issues.extend(verify_existing_npm(context, npm, manifest)) if issues: raise ReleaseError("PyPI publication is blocked:\n " + "\n ".join(issues)) - if pypi.exists: + missing_keys = missing_pypi_artifact_keys(pypi, manifest) + if not missing_keys: print( f"PyPI already has {context.package_name} {context.version.project}; verified existing files." ) smoke.post_publish_pypi_check(context, runner) return 0 - fail_if_local_artifacts_stale( - context, manifest, ("pypi_sdist", "pypi_wheel"), "PyPI" - ) - sdist = context.root / manifest.artifact("pypi_sdist").path - wheel = context.root / manifest.artifact("pypi_wheel").path + require_pypi_auth() + fail_if_local_artifacts_stale(context, manifest, missing_keys, "PyPI") command = [ sys.executable, "-m", @@ -114,7 +114,9 @@ def publish_pypi(root: Path, runner: CommandRunner, execute: bool) -> int: os.environ.get("PYPI_REPOSITORY", "pypi"), ] command.extend(shlex.split(os.environ.get("TWINE_UPLOAD_ARGS", ""))) - command.extend([str(sdist), str(wheel)]) + command.extend( + str(context.root / manifest.artifact(key).path) for key in missing_keys + ) runner.run(command, cwd=context.root, env=pypi_upload_env(), capture_output=False) issues = wait_for_registry_verification( "PyPI", @@ -137,7 +139,6 @@ def publish_npm(root: Path, runner: CommandRunner, execute: bool) -> int: return 1 manifest = read_manifest(root) fail_if_generated_metadata_stale(context, manifest) - require_npm_auth() pypi = query_pypi_release(context) npm = query_npm_release(context) require_publish_git_state( @@ -150,14 +151,17 @@ def publish_npm(root: Path, runner: CommandRunner, execute: bool) -> int: issues.extend(verify_existing_pypi(context, pypi, manifest)) if issues: raise ReleaseError("npm publication is blocked:\n " + "\n ".join(issues)) - needs_dist_tag = npm.exists is False or npm.latest != context.version.npm + needs_dist_tag = npm.exists and npm.latest != context.version.npm + if not npm.exists or needs_dist_tag: + require_npm_auth() if not npm.exists: fail_if_local_artifacts_stale(context, manifest, ("npm_tarball",), "npm") otp = resolve_npm_otp(npm_exists=npm.exists, needs_dist_tag=needs_dist_tag) if not npm.exists: package = context.root / manifest.artifact("npm_tarball").path - command = ["npm", "publish", str(package), "--tag", "latest"] + command = ["npm", "publish", str(package)] command.extend(npm_extra_args()) + command.extend(["--tag", "latest"]) command.extend(otp_arg(otp.publish)) runner.run(command, cwd=context.root, capture_output=False) if needs_dist_tag: @@ -426,7 +430,10 @@ def wait_for_registry_verification( ) -> list[str]: issues: list[str] = [] for attempt in range(1, attempts + 1): - issues = collect_issues() + try: + issues = collect_issues() + except RetryableRegistryError as error: + issues = [str(error)] if not issues: if attempt > 1: retries = attempt - 1 @@ -435,11 +442,12 @@ def wait_for_registry_verification( return [] if attempt == attempts: return issues + delay = 2 ** (attempt - 1) print( - f"{label} registry verification did not pass; " - f"retrying ({attempt}/{attempts})." + f"{label} registry verification pending ({attempt}/{attempts}): " + f"{'; '.join(issues)}; retrying in {delay}s." ) - time.sleep(2 ** (attempt - 1)) + time.sleep(delay) return issues diff --git a/scripts/release/smoke.py b/scripts/release/smoke.py index 0e3fbf3..150e973 100644 --- a/scripts/release/smoke.py +++ b/scripts/release/smoke.py @@ -20,23 +20,50 @@ def install_check(root: Path, runner: CommandRunner) -> None: + context = read_release_context(root) build.package_check(root, runner) - install_smoke(root, runner) - install_script_smoke(root, runner) - build.npm_pack(root, runner) - npm_smoke(root, runner) - brew_smoke(root, runner) + build.build_wheelhouse(context, runner) + _exercise_python_install_smokes(context, runner) + _install_script_smoke(context, runner) + if command_exists("npm"): + build.npm_pack(root, runner) + _npm_smoke(context, runner) + else: + print("Skipping npm-smoke: npm not found.") + if command_exists("brew"): + _brew_smoke(context, runner) + else: + print("Skipping brew-smoke: brew not found.") def install_smoke(root: Path, runner: CommandRunner) -> None: - install_smoke_pip(root, runner) - install_smoke_uv(root, runner) - install_smoke_pipx(root, runner) + context = read_release_context(root) + build.package_wheelhouse(root, runner) + _exercise_python_install_smokes(context, runner) + + +def _exercise_python_install_smokes( + context: ReleaseContext, runner: CommandRunner +) -> None: + _install_smoke_pip(context, runner) + if command_exists("uv"): + _install_smoke_uv(context, runner) + else: + print("Skipping install-smoke-uv: uv not found.") + if command_exists("pipx"): + _install_smoke_pipx(context, runner) + else: + print("Skipping install-smoke-pipx: pipx not found.") def install_smoke_pip(root: Path, runner: CommandRunner) -> None: context = read_release_context(root) build.package_wheelhouse(root, runner) + _install_smoke_pip(context, runner) + + +def _install_smoke_pip(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root with tempfile.TemporaryDirectory() as temporary: tmp = Path(temporary) smoke_python = current_python(root, runner) @@ -72,6 +99,11 @@ def install_smoke_uv(root: Path, runner: CommandRunner) -> None: return context = read_release_context(root) build.package_wheelhouse(root, runner) + _install_smoke_uv(context, runner) + + +def _install_smoke_uv(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root with tempfile.TemporaryDirectory() as temporary: tmp = Path(temporary) home = tmp / "home" @@ -109,6 +141,11 @@ def install_smoke_pipx(root: Path, runner: CommandRunner) -> None: return context = read_release_context(root) build.package_wheelhouse(root, runner) + _install_smoke_pipx(context, runner) + + +def _install_smoke_pipx(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root with tempfile.TemporaryDirectory() as temporary: tmp = Path(temporary) smoke_python = current_python(root, runner) @@ -133,6 +170,11 @@ def install_smoke_pipx(root: Path, runner: CommandRunner) -> None: def install_script_smoke(root: Path, runner: CommandRunner) -> None: context = read_release_context(root) build.package_wheelhouse(root, runner) + _install_script_smoke(context, runner) + + +def _install_script_smoke(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root with tempfile.TemporaryDirectory() as temporary: tmp = Path(temporary) env = { @@ -155,6 +197,11 @@ def npm_smoke(root: Path, runner: CommandRunner) -> None: context = read_release_context(root) build.package_wheelhouse(root, runner) build.npm_pack(root, runner) + _npm_smoke(context, runner) + + +def _npm_smoke(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root package = context.root / ".release" / "npm" / context.npm_filename if not package.is_file(): raise ReleaseError(f"npm package artifact is missing: {package}") @@ -205,6 +252,11 @@ def brew_smoke(root: Path, runner: CommandRunner) -> None: return context = read_release_context(root) build.package_build(root, runner) + _brew_smoke(context, runner) + + +def _brew_smoke(context: ReleaseContext, runner: CommandRunner) -> None: + root = context.root installed = runner.run( ["brew", "list", "--formula", context.package_name], cwd=root, diff --git a/scripts/release/state_checks.py b/scripts/release/state_checks.py index baeba57..bf54949 100644 --- a/scripts/release/state_checks.py +++ b/scripts/release/state_checks.py @@ -693,21 +693,36 @@ def verify_pypi_artifacts( ) -> list[str]: if not release.exists: return ["PyPI version is missing"] + issues = verify_present_pypi_artifacts(context, release, manifest) + for key in missing_pypi_artifact_keys(release, manifest): + issues.append(f"PyPI is missing {manifest.artifact(key).filename}") + return issues + + +def missing_pypi_artifact_keys( + release: PypiRelease, manifest: ReleaseManifest +) -> tuple[str, ...]: + return tuple( + key + for key in ("pypi_sdist", "pypi_wheel") + if manifest.artifact(key).filename not in release.files + ) + + +def verify_present_pypi_artifacts( + context: ReleaseContext, release: PypiRelease, manifest: ReleaseManifest +) -> list[str]: issues: list[str] = [] for key in ("pypi_sdist", "pypi_wheel"): artifact = manifest.artifact(key) remote = release.files.get(artifact.filename) if remote is None: - issues.append(f"PyPI is missing {artifact.filename}") continue if remote.sha256 != artifact.sha256: issues.append(f"PyPI hash mismatch for {artifact.filename}") if remote.size and remote.size != artifact.size: issues.append(f"PyPI size mismatch for {artifact.filename}") expected_names = {context.sdist_filename, context.wheel_filename} - for filename in expected_names: - if filename not in release.files: - issues.append(f"PyPI is missing expected file {filename}") unexpected_names = sorted(release.files.keys() - expected_names) if unexpected_names: issues.append(f"PyPI has unexpected files: {', '.join(unexpected_names)}") diff --git a/scripts/release/state_state.py b/scripts/release/state_state.py index 0bb46f6..6305f2c 100644 --- a/scripts/release/state_state.py +++ b/scripts/release/state_state.py @@ -4,8 +4,9 @@ _verify_required_homebrew_resource_specs as _verify_homebrew_specs, ) from .state_checks import ( + missing_pypi_artifact_keys, verify_npm_artifact, - verify_pypi_artifacts, + verify_present_pypi_artifacts, ) from .state_types import ( DerivedReleaseState, @@ -49,6 +50,7 @@ def derive_release_state( ) artifact_issues: list[str] = [] + missing_pypi_keys: tuple[str, ...] = () if pypi.exists or npm.exists: if manifest is None: return DerivedReleaseState( @@ -59,7 +61,10 @@ def derive_release_state( ), ) if pypi.exists: - artifact_issues.extend(verify_pypi_artifacts(context, pypi, manifest)) + artifact_issues.extend( + verify_present_pypi_artifacts(context, pypi, manifest) + ) + missing_pypi_keys = missing_pypi_artifact_keys(pypi, manifest) if npm.exists: artifact_issues.extend(verify_npm_artifact(context, npm, manifest)) if artifact_issues: @@ -71,19 +76,28 @@ def derive_release_state( tuple(reasons), ( "Remote registry artifacts do not match local manifest; " - "recover manually before rerunning release commands." + "recover manually before rerunning release commands.", ), ) formula_issues = verify_formula_state_for_release(context, formula, manifest) tag_issues = verify_git_tag_state(git) - if pypi.exists and npm.exists and npm.latest == context.version.npm: + pypi_complete = pypi.exists and not missing_pypi_keys + missing_pypi_issues = [ + f"PyPI is missing {manifest.artifact(key).filename}" + for key in missing_pypi_keys + if manifest is not None + ] + + if pypi_complete and npm.exists and npm.latest == context.version.npm: if formula_issues or tag_issues: reasons.extend(formula_issues) reasons.extend(tag_issues) guidance.extend( - guidance_for_missing_side_effects(False, False, npm.latest, context) + guidance_for_missing_side_effects( + pypi_complete, npm.exists, npm.latest, context + ) ) return DerivedReleaseState( ReleaseStatus.PARTIAL, tuple(reasons), tuple(guidance) @@ -97,11 +111,12 @@ def derive_release_state( ) if pypi.exists or npm.exists: + reasons.extend(missing_pypi_issues) reasons.extend(formula_issues) reasons.extend(tag_issues) guidance.extend( guidance_for_missing_side_effects( - pypi.exists, npm.exists, npm.latest, context + pypi_complete, npm.exists, npm.latest, context ) ) return DerivedReleaseState( @@ -109,7 +124,11 @@ def derive_release_state( ) reasons.extend(formula_issues) - reasons.extend(tag_issues) + tag_is_expectedly_absent = ( + git is not None and not git.tag_commit and not git.remote_tag_commit + ) + if not tag_is_expectedly_absent: + reasons.extend(tag_issues) return DerivedReleaseState(ReleaseStatus.READY, tuple(reasons), tuple(guidance)) @@ -132,7 +151,25 @@ def manifest_context_issues( } if actual != expected: return ["release manifest package identity does not match pyproject.toml"] - return [] + return _pypi_manifest_filename_issues(context, manifest) + + +def _pypi_manifest_filename_issues( + context: ReleaseContext, manifest: ReleaseManifest +) -> list[str]: + expected_filenames = { + "pypi_sdist": context.sdist_filename, + "pypi_wheel": context.wheel_filename, + } + issues: list[str] = [] + for key, expected_filename in expected_filenames.items(): + actual_filename = manifest.artifact(key).filename + if actual_filename != expected_filename: + issues.append( + f"release manifest {key} filename does not match release context: " + f"expected {expected_filename}, found {actual_filename}" + ) + return issues def verify_formula_state_for_release( diff --git a/scripts/release/state_types.py b/scripts/release/state_types.py index cb30999..94ccd99 100644 --- a/scripts/release/state_types.py +++ b/scripts/release/state_types.py @@ -29,6 +29,10 @@ class ReleaseError(RuntimeError): """Raised when release state is unsafe or cannot be verified.""" +class RetryableRegistryError(ReleaseError): + """Raised when a registry query may succeed on a later attempt.""" + + class ReleaseStatus(StrEnum): READY = "ready" COMPLETE = "complete" @@ -84,27 +88,42 @@ def run( def _redacted_args(command: Sequence[str]) -> tuple[str, ...]: redacted: list[str] = [] - previous = "" + secret_option = "" for arg in command: - if previous == "--otp": + option, separator, _value = arg.partition("=") + if secret_option: redacted.append("") - previous = "" + secret_option = ( + option if _is_secret_option(option) and not separator else "" + ) continue - if arg == "--otp": - redacted.append(arg) - previous = "--otp" + if arg.startswith("-p") and not arg.startswith("--") and len(arg) > 2: + redacted.append("-p") continue - if arg.startswith("--otp="): - redacted.append("--otp=") - previous = "" + if _is_secret_option(option): + if separator: + redacted.append(f"{option}=") + continue + redacted.append(arg) + secret_option = option continue - previous = "" redacted.append(arg) - if previous == "--otp": - redacted.append("") + if secret_option: + redacted.append("") return tuple(redacted) +def _is_secret_option(option: str) -> bool: + normalized = option.casefold() + return normalized in { + "--otp", + "--password", + "-p", + "--token", + "--auth-token", + } or normalized.endswith("_authtoken") + + def _command_failure_output(result: CommandResult) -> str: sections: list[str] = [] if result.stdout.strip(): @@ -405,11 +424,18 @@ def fetch_registry_json(url: str) -> dict[str, Any] | None: except urllib.error.HTTPError as error: if error.code == 404: return None - raise ReleaseError( + error_type = ( + RetryableRegistryError + if error.code in {408, 429} or 500 <= error.code < 600 + else ReleaseError + ) + raise error_type( f"registry query failed for {url}: HTTP {error.code}" ) from error except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as error: - raise ReleaseError(f"registry query failed for {url}: {error}") from error + raise RetryableRegistryError( + f"registry query failed for {url}: {error}" + ) from error if not isinstance(payload, dict): raise ReleaseError(f"registry response is not a JSON object: {url}") return payload diff --git a/tests/unit/packaging/test_release_surfaces.py b/tests/unit/packaging/test_release_surfaces.py index c4f2396..36b66ae 100644 --- a/tests/unit/packaging/test_release_surfaces.py +++ b/tests/unit/packaging/test_release_surfaces.py @@ -549,74 +549,28 @@ def test_release_docs_describe_single_production_publish_path() -> None: development = read_text("DEVELOPMENT.md") contributing = read_text("CONTRIBUTING.md") normalized_development = " ".join(development.split()) - normalized_contributing = " ".join(contributing.split()) assert "Production publishing is local-only." in development - assert "Maintainers run `make release`" in normalized_development + assert "[Release Workflow](#release-workflow)" in development assert "does not publish production PyPI or npm packages" in normalized_development - assert "does not need PyPI or npm credentials" in normalized_development - assert "manually dispatched GitHub Release automation" in normalized_development - assert "required `tag` input" in normalized_development - assert "rejects dispatches outside `refs/heads/master`" in normalized_development - assert "manual `make release` flow is the production source of truth" in ( - normalized_development - ) - assert "second phase of the same release operation" in normalized_development - assert ( - "requires it to match the master commit that dispatched the workflow exactly" - in normalized_development - ) - assert ( - "Historical-tag backfills and new dispatches after `master` advances are unsupported" - in normalized_development - ) - assert "exact Hatchling build-system pin" in normalized_development - assert "offline runtime wheelhouse" in normalized_development - assert "GitHub Release itself contains only `dist/*`" in (normalized_development) - assert "New releases are built as drafts, verified, published" in ( - normalized_development - ) - assert "exact name, size, and GitHub's immutable upload-time SHA-256 digest" in ( - normalized_development - ) + assert "does not need their credentials" in normalized_development + assert "## Release Workflow" in development + for command in ( + "make release-prepare", + "make release-check", + "make release", + "make release-pypi", + "make release-npm", + ): + assert command in development + assert "`v0.1.4`, not `0.1.4`" in normalized_development + assert "before `master` advances" in normalized_development + assert "backfilling a historical tag" in normalized_development assert "highest published stable version on PyPI" in normalized_development - assert "does not update the Homebrew tap" in development - assert "TestPyPI Trusted Publishing workflow" in normalized_development - assert "dispatch it from any selected ref" in normalized_development - assert "not restricted to `master`" in normalized_development - assert "delayed GitHub Release" not in normalized_development - assert "stale attempts for older releases" not in normalized_development - assert "PyPI Trusted Publishing through GitHub OIDC" not in development - assert "scripts/release.py release-artifacts" in contributing - assert "scripts/release.py github-release-plan" in contributing - assert "recover-release-artifacts" not in contributing - assert "verify-backfill" not in contributing - assert "exact Hatchling build-system pin" in normalized_contributing - assert "offline runtime wheelhouse" in normalized_contributing - assert "exact asset names, sizes, and GitHub SHA-256 digests" in ( - normalized_contributing - ) - assert "never mutates an already-published mismatch" in (normalized_contributing) - assert "highest published stable version on PyPI" in normalized_contributing - assert "checks out the requested tag" in normalized_contributing - assert ( - "requires it to match the master commit that dispatched the workflow exactly" - in normalized_contributing - ) - assert ( - "Historical-tag backfills and new dispatches after `master` advances are unsupported" - in normalized_contributing - ) - assert "checks out the exact verified commit" in normalized_contributing - assert "current `master` tip" not in normalized_contributing - assert "reloads and re-verifies draft assets immediately before publication" in ( - normalized_contributing - ) - assert "does not publish production packages" in normalized_contributing - assert "dispatch it from any selected ref" in normalized_contributing - assert "not restricted to `master`" in normalized_contributing - assert "delayed GitHub Release" not in normalized_contributing - assert "stale attempts for older releases" not in normalized_contributing + assert "Publish Homebrew separately" in development + assert ".github/workflows/testpypi.yml" in development + assert "dispatched from any selected ref" in normalized_development + assert "## Release Workflow" not in contributing def test_repository_automation_matches_supported_platform_and_publish_policy() -> None: diff --git a/tests/unit/packaging/test_release_tool_publish.py b/tests/unit/packaging/test_release_tool_publish.py index ad9ead3..220a062 100644 --- a/tests/unit/packaging/test_release_tool_publish.py +++ b/tests/unit/packaging/test_release_tool_publish.py @@ -2,6 +2,7 @@ # ruff: noqa: E402, I001 +from dataclasses import replace import sys from pathlib import Path @@ -710,7 +711,113 @@ def query_pypi(context_arg: state.ReleaseContext) -> state.PypiRelease: assert "PyPI registry verification passed after 1 retry." in capsys.readouterr().out -def test_publish_npm_retries_registry_visibility_after_publish( +def test_publish_pypi_recovers_by_uploading_only_missing_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + context, manifest, _formula, git = release_state_fixture(tmp_path) + write_manifest(tmp_path, manifest) + sdist = manifest.artifact("pypi_sdist") + partial_release = state.PypiRelease( + True, + context.version.python, + { + sdist.filename: state.PypiFile( + sdist.filename, + sdist.size, + sdist.sha256, + ) + }, + ) + pypi_responses = iter([partial_release, matching_pypi(context, manifest)]) + checked_keys: list[tuple[str, ...]] = [] + + def query_pypi(context_arg: state.ReleaseContext) -> state.PypiRelease: + del context_arg + return next(pypi_responses) + + def capture_checked_keys( + context_arg: state.ReleaseContext, + manifest_arg: state.ReleaseManifest, + keys: tuple[str, ...], + label: str, + ) -> None: + del context_arg, manifest_arg, label + checked_keys.append(keys) + + monkeypatch.setattr(publish, "read_release_context", constant(context)) + monkeypatch.setattr(publish, "read_manifest", constant(manifest)) + monkeypatch.setattr(publish, "fail_if_generated_metadata_stale", no_op) + monkeypatch.setattr(publish, "require_pypi_auth", no_op) + monkeypatch.setattr(publish, "require_publish_git_state", constant(git)) + monkeypatch.setattr(publish.smoke, "post_publish_pypi_check", no_op) + monkeypatch.setattr( + publish, + "query_pypi_release", + query_pypi, + ) + monkeypatch.setattr( + publish, + "query_npm_release", + constant(state.NpmRelease(False, "", "", "", "", "", "", "", "")), + ) + monkeypatch.setattr( + publish, + "fail_if_local_artifacts_stale", + capture_checked_keys, + ) + runner = FakeRunner() + + assert publish.publish_pypi(tmp_path, runner, execute=True) == 0 + + wheel_path = str(tmp_path / manifest.artifact("pypi_wheel").path) + sdist_path = str(tmp_path / manifest.artifact("pypi_sdist").path) + assert checked_keys == [("pypi_wheel",)] + assert len(runner.commands) == 1 + assert wheel_path in runner.commands[0] + assert sdist_path not in runner.commands[0] + + +def test_publish_pypi_rejects_noncanonical_manifest_filename_before_queries( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + context, manifest, _formula, _git = release_state_fixture(tmp_path) + state.sync_generated_metadata(context, FakeRunner()) + state.sync_homebrew_formula_metadata( + context, manifest.artifact("pypi_sdist").sha256 + ) + wheel = manifest.artifact("pypi_wheel") + alternate_filename = wheel.filename.replace( + "-py3-none-any.whl", "-1-py3-none-any.whl" + ) + alternate_wheel = replace( + wheel, + path=f"dist/{alternate_filename}", + filename=alternate_filename, + ) + mismatched_manifest = replace( + manifest, + artifacts={**manifest.artifacts, "pypi_wheel": alternate_wheel}, + ) + write_manifest(tmp_path, mismatched_manifest) + + def fail_registry_query(context_arg: state.ReleaseContext) -> None: + del context_arg + pytest.fail("manifest validation must run before registry queries") + + monkeypatch.setattr(publish, "query_pypi_release", fail_registry_query) + monkeypatch.setattr(publish, "query_npm_release", fail_registry_query) + runner = FakeRunner() + + with pytest.raises( + state.ReleaseError, + match="pypi_wheel filename does not match release context", + ): + publish.publish_pypi(tmp_path, runner, execute=True) + + assert not runner.commands + + +def test_publish_npm_enforces_latest_and_retries_registry_visibility( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: context, manifest, _formula, git = release_state_fixture(tmp_path) @@ -724,11 +831,14 @@ def test_publish_npm_retries_registry_visibility_after_publish( monkeypatch.setattr(publish, "read_release_context", constant(context)) monkeypatch.setattr(publish, "read_manifest", constant(manifest)) monkeypatch.setattr(publish, "fail_if_generated_metadata_stale", no_op) + monkeypatch.setenv("NPM_PUBLISH_ARGS", "--tag next") monkeypatch.setattr(publish, "require_npm_auth", lambda: None) monkeypatch.setattr(publish, "require_publish_git_state", constant(git)) monkeypatch.setattr(publish, "fail_if_local_artifacts_stale", no_op) monkeypatch.setattr(publish.smoke, "post_publish_npm_check", no_op) - monkeypatch.setattr(publish, "resolve_npm_otp", constant(publish.NpmOtp("", ""))) + monkeypatch.setattr( + publish, "resolve_npm_otp", constant(publish.NpmOtp("222222", "")) + ) monkeypatch.setattr(publish.time, "sleep", lambda seconds: sleeps.append(seconds)) monkeypatch.setattr( publish, @@ -742,12 +852,65 @@ def query_npm(context_arg: state.ReleaseContext) -> state.NpmRelease: monkeypatch.setattr(publish, "query_npm_release", query_npm) - assert publish.publish_npm(tmp_path, FakeRunner(), execute=True) == 0 + runner = FakeRunner() + assert publish.publish_npm(tmp_path, runner, execute=True) == 0 assert sleeps == [1] assert not npm_responses + npm_mutations = [command for command in runner.commands if command[0] == "npm"] + assert len(npm_mutations) == 1 + assert npm_mutations[0][:2] == ("npm", "publish") + assert npm_mutations[0][-3:] == ("--tag", "latest", "--otp=222222") assert "npm registry verification passed after 1 retry." in capsys.readouterr().out +def test_publish_npm_reconciles_stale_latest_without_republishing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + context, manifest, _formula, git = release_state_fixture(tmp_path) + write_manifest(tmp_path, manifest) + npm_responses = iter( + [ + matching_npm(context, manifest, latest="previous"), + matching_npm(context, manifest, latest=context.version.npm), + ] + ) + + def query_npm(context_arg: state.ReleaseContext) -> state.NpmRelease: + del context_arg + return next(npm_responses) + + monkeypatch.setattr(publish, "read_release_context", constant(context)) + monkeypatch.setattr(publish, "read_manifest", constant(manifest)) + monkeypatch.setattr(publish, "fail_if_generated_metadata_stale", no_op) + monkeypatch.setattr(publish, "require_npm_auth", no_op) + monkeypatch.setattr(publish, "require_publish_git_state", constant(git)) + monkeypatch.setattr(publish.smoke, "post_publish_npm_check", no_op) + monkeypatch.setattr( + publish, + "resolve_npm_otp", + constant(publish.NpmOtp("", "444444")), + ) + monkeypatch.setattr( + publish, + "query_pypi_release", + constant(matching_pypi(context, manifest)), + ) + monkeypatch.setattr(publish, "query_npm_release", query_npm) + runner = FakeRunner() + + assert publish.publish_npm(tmp_path, runner, execute=True) == 0 + assert runner.commands == [ + ( + "npm", + "dist-tag", + "add", + f"{context.package_name}@{context.version.npm}", + "latest", + "--otp=444444", + ) + ] + + def test_registry_verification_success_message_uses_plural_retries( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: @@ -768,6 +931,53 @@ def test_registry_verification_success_message_uses_plural_retries( ) +def test_registry_verification_retries_transient_query_error( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + calls = 0 + sleeps: list[int] = [] + + def collect_issues() -> list[str]: + nonlocal calls + calls += 1 + if calls == 1: + raise state.RetryableRegistryError("registry query failed: HTTP 503") + return [] + + monkeypatch.setattr(publish.time, "sleep", lambda seconds: sleeps.append(seconds)) + + assert ( + publish.wait_for_registry_verification("npm", collect_issues, attempts=2) == [] + ) + assert calls == 2 + assert sleeps == [1] + output = capsys.readouterr().out + assert "registry query failed: HTTP 503" in output + assert "retrying in 1s" in output + + +def test_registry_verification_does_not_retry_non_transient_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def fail_sleep(seconds: int) -> None: + del seconds + pytest.fail("non-transient failures must not be retried") + + monkeypatch.setattr( + publish.time, + "sleep", + fail_sleep, + ) + + with pytest.raises(state.ReleaseError, match="malformed registry response"): + publish.wait_for_registry_verification( + "npm", + lambda: (_ for _ in ()).throw( + state.ReleaseError("malformed registry response") + ), + ) + + def test_npm_otp_handling_requires_independent_non_tty_values( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -775,6 +985,10 @@ def test_npm_otp_handling_requires_independent_non_tty_values( monkeypatch.setenv("NPM_OTP", "111111") monkeypatch.delenv("NPM_PUBLISH_OTP", raising=False) monkeypatch.delenv("NPM_DIST_TAG_OTP", raising=False) + otp = publish.resolve_npm_otp(npm_exists=False, needs_dist_tag=False) + assert otp.publish == "111111" + assert otp.dist_tag == "" + with pytest.raises(state.ReleaseError, match="separate OTP"): publish.resolve_npm_otp(npm_exists=False, needs_dist_tag=True) diff --git a/tests/unit/packaging/test_release_tool_smoke.py b/tests/unit/packaging/test_release_tool_smoke.py index eb4f765..1c801a7 100644 --- a/tests/unit/packaging/test_release_tool_smoke.py +++ b/tests/unit/packaging/test_release_tool_smoke.py @@ -62,3 +62,70 @@ def fake_package_build(root: Path, runner: BrewSmokeRunner) -> None: assert f'url "file://{sdist}"' in runner.installed_formula_text assert f'sha256 "{expected_sha}"' in runner.installed_formula_text assert f'sha256 "{"0" * 64}"' not in runner.installed_formula_text + + +def test_install_check_prepares_each_artifact_set_once( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_minimal_repo(tmp_path) + build_steps: list[str] = [] + smoke_steps: list[str] = [] + + def record_step(target: list[str], step: str): + def record(*args) -> None: + del args + target.append(step) + + return record + + monkeypatch.setattr( + smoke.build, + "package_check", + record_step(build_steps, "python"), + ) + monkeypatch.setattr( + smoke.build, + "build_wheelhouse", + record_step(build_steps, "wheelhouse"), + ) + monkeypatch.setattr( + smoke.build, + "npm_pack", + record_step(build_steps, "npm"), + ) + monkeypatch.setattr(smoke, "command_exists", lambda name: bool(name)) + monkeypatch.setattr( + smoke, + "_install_smoke_pip", + record_step(smoke_steps, "pip"), + ) + monkeypatch.setattr( + smoke, + "_install_smoke_uv", + record_step(smoke_steps, "uv"), + ) + monkeypatch.setattr( + smoke, + "_install_smoke_pipx", + record_step(smoke_steps, "pipx"), + ) + monkeypatch.setattr( + smoke, + "_install_script_smoke", + record_step(smoke_steps, "install-script"), + ) + monkeypatch.setattr( + smoke, + "_npm_smoke", + record_step(smoke_steps, "npm"), + ) + monkeypatch.setattr( + smoke, + "_brew_smoke", + record_step(smoke_steps, "brew"), + ) + + smoke.install_check(tmp_path, state.CommandRunner()) + + assert build_steps == ["python", "wheelhouse", "npm"] + assert smoke_steps == ["pip", "uv", "pipx", "install-script", "npm", "brew"] diff --git a/tests/unit/packaging/test_release_tool_state.py b/tests/unit/packaging/test_release_tool_state.py index a2ddc14..a6d7853 100644 --- a/tests/unit/packaging/test_release_tool_state.py +++ b/tests/unit/packaging/test_release_tool_state.py @@ -5,6 +5,7 @@ import json from pathlib import Path import sys +import urllib.error _LOCAL_TEST_DIR = Path(__file__).resolve().parent if str(_LOCAL_TEST_DIR) not in sys.path: @@ -73,6 +74,87 @@ def test_command_runner_reports_stdout_and_stderr_on_failure(tmp_path: Path) -> assert "stderr:\nvisible stderr" in message +def test_command_runner_redacts_credentials_from_failure( + tmp_path: Path, +) -> None: + runner = state.CommandRunner() + + with pytest.raises(state.ReleaseError) as caught: + runner.run( + [ + sys.executable, + "-c", + "raise SystemExit(7)", + "--password", + "pypi-secret", + "--//registry.npmjs.org/:_authToken=npm-secret", + "--otp=123456", + "-pattached-secret", + "--_authToken=generic-secret", + ], + cwd=tmp_path, + ) + + message = str(caught.value) + assert "pypi-secret" not in message + assert "npm-secret" not in message + assert "123456" not in message + assert "attached-secret" not in message + assert "generic-secret" not in message + assert message.count("") == 5 + + +def test_command_runner_redacts_adjacent_secret_options(tmp_path: Path) -> None: + secret = "nested-secret" + + with pytest.raises(state.ReleaseError) as caught: + state.CommandRunner().run( + [ + sys.executable, + "-c", + "raise SystemExit(7)", + "-p", + "--otp", + secret, + ], + cwd=tmp_path, + ) + + assert secret not in str(caught.value) + + +@pytest.mark.parametrize( + ("status_code", "error_type"), + ( + (408, state.RetryableRegistryError), + (429, state.RetryableRegistryError), + (503, state.RetryableRegistryError), + (400, state.ReleaseError), + ), +) +def test_registry_http_errors_are_classified_for_retry( + monkeypatch: pytest.MonkeyPatch, + status_code: int, + error_type: type[state.ReleaseError], +) -> None: + def fail_request(*args, **kwargs): + del args, kwargs + raise urllib.error.HTTPError( + "https://registry.example.test/package", + status_code, + "failure", + None, + None, + ) + + monkeypatch.setattr(state.state_types.urllib.request, "urlopen", fail_request) + + with pytest.raises(state.ReleaseError) as caught: + state.fetch_registry_json("https://registry.example.test/package") + + assert type(caught.value) is error_type + + def test_metadata_sync_refreshes_and_reads_back_generated_files( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/unit/packaging/test_release_tool_state_checks.py b/tests/unit/packaging/test_release_tool_state_checks.py index 64a7b4d..b9d6729 100644 --- a/tests/unit/packaging/test_release_tool_state_checks.py +++ b/tests/unit/packaging/test_release_tool_state_checks.py @@ -2,6 +2,7 @@ # ruff: noqa: E402, I001 +from dataclasses import replace from pathlib import Path import subprocess import sys @@ -40,6 +41,19 @@ def test_release_state_derivation_ready_complete_partial_and_blocked( ) assert ready.status == state.ReleaseStatus.READY + tag_missing_git = replace(git, tag_commit="", remote_tag_commit="") + ready_before_tagging = state.derive_release_state( + context, missing_pypi, missing_npm, formula, tag_missing_git, manifest + ) + assert ready_before_tagging.status == state.ReleaseStatus.READY + assert "Git tag is missing locally or on origin" not in ready_before_tagging.reasons + + local_tag_only = replace(git, remote_tag_commit="") + ready_with_partial_tag = state.derive_release_state( + context, missing_pypi, missing_npm, formula, local_tag_only, manifest + ) + assert "Git tag is missing locally or on origin" in ready_with_partial_tag.reasons + unreachable_git = state.GitState( branch="release-fix", default_branch="master", @@ -70,6 +84,40 @@ def test_release_state_derivation_ready_complete_partial_and_blocked( assert partial.status == state.ReleaseStatus.PARTIAL assert any("release-npm" in item for item in partial.guidance) + partial_before_tagging = state.derive_release_state( + context, pypi, missing_npm, formula, tag_missing_git, manifest + ) + assert "Git tag is missing locally or on origin" in partial_before_tagging.reasons + + finalization_pending = state.derive_release_state( + context, pypi, npm, formula, tag_missing_git, manifest + ) + assert finalization_pending.guidance == ( + "Rerun make release after fixing Git tag or Homebrew formula state.", + ) + + sdist = manifest.artifact("pypi_sdist") + partial_pypi = state.PypiRelease( + True, + context.version.python, + { + sdist.filename: state.PypiFile( + sdist.filename, + sdist.size, + sdist.sha256, + ) + }, + ) + recoverable_pypi = state.derive_release_state( + context, partial_pypi, npm, formula, tag_missing_git, manifest + ) + assert recoverable_pypi.status == state.ReleaseStatus.PARTIAL + assert ( + recoverable_pypi.reasons.count(f"PyPI is missing {context.wheel_filename}") == 1 + ) + assert any("release-pypi" in item for item in recoverable_pypi.guidance) + assert not any("release-npm" in item for item in recoverable_pypi.guidance) + bad_pypi = state.PypiRelease( True, context.version.python, @@ -88,6 +136,10 @@ def test_release_state_derivation_ready_complete_partial_and_blocked( context, bad_pypi, missing_npm, formula, git, manifest ) assert blocked.status == state.ReleaseStatus.BLOCKED + assert blocked.guidance == ( + "Remote registry artifacts do not match local manifest; " + "recover manually before rerunning release commands.", + ) def test_complete_state_requires_formula_resources_to_match_requirements( @@ -238,6 +290,150 @@ def fail_suite(_root: Path, _runner: FakeRunner) -> None: assert release_script.release_check(tmp_path, FakeRunner()) == 0 +def test_release_check_does_not_treat_partial_pypi_as_tag_only( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + release_script = load_release_script() + context, manifest, formula, git = release_state_fixture(tmp_path) + sdist = manifest.artifact("pypi_sdist") + partial_pypi = state.PypiRelease( + True, + context.version.python, + { + sdist.filename: state.PypiFile( + sdist.filename, + sdist.size, + sdist.sha256, + ) + }, + ) + npm = matching_npm(context, manifest, latest=context.version.npm) + tag_missing_git = replace(git, tag_commit="", remote_tag_commit="") + monkeypatch.setattr(release_script, "read_release_context", constant(context)) + monkeypatch.setattr(release_script, "read_manifest_if_present", constant(manifest)) + monkeypatch.setattr( + release_script, "query_registry_state", constant((partial_pypi, npm)) + ) + monkeypatch.setattr(release_script, "read_formula_state", constant(formula)) + monkeypatch.setattr(release_script, "inspect_git_state", constant(tag_missing_git)) + + assert release_script.release_check(tmp_path, FakeRunner()) == 1 + + +def test_release_check_requires_current_changelog_section_before_suite( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + release_script = load_release_script() + context, manifest, formula, git = release_state_fixture(tmp_path) + missing_pypi = state.PypiRelease(False, "", {}) + missing_npm = state.NpmRelease(False, "", "", "", "", "", "", "", "") + tag_missing_git = replace(git, tag_commit="", remote_tag_commit="") + (tmp_path / "CHANGELOG.md").write_text( + f"# Changelog\n\n## {context.version.project}0\n", + encoding="utf-8", + ) + monkeypatch.setattr(release_script, "read_release_context", constant(context)) + monkeypatch.setattr(release_script, "read_manifest_if_present", constant(manifest)) + monkeypatch.setattr( + release_script, "query_registry_state", constant((missing_pypi, missing_npm)) + ) + monkeypatch.setattr(release_script, "read_formula_state", constant(formula)) + monkeypatch.setattr(release_script, "inspect_git_state", constant(tag_missing_git)) + monkeypatch.setattr(release_script, "fail_if_generated_metadata_stale", no_op) + + def fail_suite(_root: Path, _runner: FakeRunner) -> None: + del _root, _runner + raise AssertionError("pre-publish suite should not run") + + monkeypatch.setattr(release_script, "run_pre_publish_checks", fail_suite) + + with pytest.raises(release_script.ReleaseError, match="does not contain a section"): + release_script.release_check(tmp_path, FakeRunner()) + + +def test_release_check_accepts_dated_current_changelog_heading( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + release_script = load_release_script() + context, manifest, formula, git = release_state_fixture(tmp_path) + missing_pypi = state.PypiRelease(False, "", {}) + missing_npm = state.NpmRelease(False, "", "", "", "", "", "", "", "") + tag_missing_git = replace(git, tag_commit="", remote_tag_commit="") + (tmp_path / "CHANGELOG.md").write_text( + f"# Changelog\n\n## [{context.version.project}] - 2026-07-27\n", + encoding="utf-8", + ) + monkeypatch.setattr(release_script, "read_release_context", constant(context)) + monkeypatch.setattr(release_script, "read_manifest_if_present", constant(manifest)) + monkeypatch.setattr( + release_script, "query_registry_state", constant((missing_pypi, missing_npm)) + ) + monkeypatch.setattr(release_script, "read_formula_state", constant(formula)) + monkeypatch.setattr(release_script, "inspect_git_state", constant(tag_missing_git)) + monkeypatch.setattr(release_script, "fail_if_generated_metadata_stale", no_op) + suites: list[Path] = [] + + def record_suite(root: Path, runner: FakeRunner) -> None: + del runner + suites.append(root) + + monkeypatch.setattr( + release_script, + "run_pre_publish_checks", + record_suite, + ) + + assert release_script.release_check(tmp_path, FakeRunner()) == 0 + assert suites == [tmp_path] + + +def test_changelog_check_accepts_linked_version_heading(tmp_path: Path) -> None: + release_script = load_release_script() + context, _manifest, _formula, _git = release_state_fixture(tmp_path) + (tmp_path / "CHANGELOG.md").write_text( + f"# Changelog\n\n## [{context.version.project}]" + f"(https://example.test/releases/{context.version.tag}) - 2026-07-27\n", + encoding="utf-8", + ) + + release_script.changelog_check(tmp_path) + + +@pytest.mark.parametrize("indentation", ("", " ", " ", " ")) +def test_changelog_check_accepts_valid_heading_indentation( + tmp_path: Path, indentation: str +) -> None: + release_script = load_release_script() + context, _manifest, _formula, _git = release_state_fixture(tmp_path) + (tmp_path / "CHANGELOG.md").write_text( + f"# Changelog\n\n{indentation}## [{context.version.project}] - 2026-07-27\n", + encoding="utf-8", + ) + + release_script.changelog_check(tmp_path) + + +@pytest.mark.parametrize( + "hidden_section", + ( + "```markdown\n## [{version}]\n```\n", + "\n", + ), +) +def test_changelog_check_rejects_hidden_version_headings( + tmp_path: Path, hidden_section: str +) -> None: + release_script = load_release_script() + context, _manifest, _formula, _git = release_state_fixture(tmp_path) + changelog = "# Changelog\n\n" + hidden_section.format( + version=context.version.project + ) + (tmp_path / "CHANGELOG.md").write_text(changelog, encoding="utf-8") + + with pytest.raises(release_script.ReleaseError, match="does not contain a section"): + release_script.changelog_check(tmp_path) + + def test_publish_pypi_checks_git_state_with_existing_pypi_release( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -566,6 +762,40 @@ def test_derive_release_state_is_blocked_on_manifest_identity_mismatch( ) +def test_derive_release_state_blocks_noncanonical_pypi_manifest_filename( + tmp_path: Path, +) -> None: + context, manifest, formula, git = release_state_fixture(tmp_path) + wheel = manifest.artifact("pypi_wheel") + alternate_filename = wheel.filename.replace( + "-py3-none-any.whl", "-1-py3-none-any.whl" + ) + alternate_wheel = replace( + wheel, + path=f"dist/{alternate_filename}", + filename=alternate_filename, + ) + mismatched_manifest = replace( + manifest, + artifacts={**manifest.artifacts, "pypi_wheel": alternate_wheel}, + ) + + state_result = state.derive_release_state( + context, + matching_pypi(context, manifest), + matching_npm(context, manifest, latest=context.version.npm), + formula, + git, + mismatched_manifest, + ) + + assert state_result.status == state.ReleaseStatus.BLOCKED + assert state_result.reasons == ( + "release manifest pypi_wheel filename does not match release context: " + f"expected {context.wheel_filename}, found {alternate_filename}", + ) + + def test_artifact_identity_checks_detect_matching_and_mismatching_registries( tmp_path: Path, ) -> None: @@ -612,6 +842,28 @@ def test_pypi_artifact_verification_rejects_unexpected_files( ) == [f"PyPI has unexpected files: {unexpected_filename}"] +def test_pypi_artifact_verification_reports_missing_file_once( + tmp_path: Path, +) -> None: + context, manifest, _formula, _git = release_state_fixture(tmp_path) + sdist = manifest.artifact("pypi_sdist") + partial_release = state.PypiRelease( + True, + context.version.python, + { + sdist.filename: state.PypiFile( + sdist.filename, + sdist.size, + sdist.sha256, + ) + }, + ) + + assert state.verify_pypi_artifacts(context, partial_release, manifest) == [ + f"PyPI is missing {context.wheel_filename}" + ] + + def test_release_notes_predecessor_uses_target_history_for_delayed_release( tmp_path: Path, ) -> None: From 63fa1274a57bef41166fab487fa4c13068fcec6a Mon Sep 17 00:00:00 2001 From: Carson Zhang <33881108+zmcddn@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:57:50 -0700 Subject: [PATCH 2/2] Fix unnecessary lambda in test Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tests/unit/packaging/test_release_tool_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/packaging/test_release_tool_smoke.py b/tests/unit/packaging/test_release_tool_smoke.py index 1c801a7..3e01440 100644 --- a/tests/unit/packaging/test_release_tool_smoke.py +++ b/tests/unit/packaging/test_release_tool_smoke.py @@ -93,7 +93,7 @@ def record(*args) -> None: "npm_pack", record_step(build_steps, "npm"), ) - monkeypatch.setattr(smoke, "command_exists", lambda name: bool(name)) + monkeypatch.setattr(smoke, "command_exists", bool) monkeypatch.setattr( smoke, "_install_smoke_pip",