Republish repaired prek caches and keep unrelated host caches

[zozo123]

Preserve repairs made to restored prek environments and avoid clearing unrelated host caches. A repaired pull-request cache is republished by an eligible writer, so subsequent jobs can reuse it; a healthy pull-request hit still skips compression and upload.

The existing refresh behavior for all non-PR runs (including push, schedule, and manual runs), two-day retention, and upstream compression settings are preserved. Readers never upload. Missing or unusable archives are treated as misses, and uncertain repair detection conservatively saves after successful installation.

Repair detection compares shallow prek installation markers across the existing bounded installation retries. Namespace v13 avoids older archives containing manual-only skill-evaluation environments; those environments remain excluded as on current main. Phase timings and save reasons remain available for CI evaluation.

Validation: 24 focused tests pass, including push/manual refresh coverage and real prek 0.5.2 repair → archive → restore → reuse. Regular static checks, scripts mypy, Ruff, and applicable manual checks pass.

The earlier warm-cache run and cold-cache run used different v12 configurations. They demonstrate cache paths, not a controlled speedup for this revision. No numerical performance improvement is claimed.

Split from #73124.

---

Was generative AI tooling used to co-author this PR?

• Yes — Codex (GPT-6)

Generated-by: Codex (GPT-6) following the guidelines

[potiuk]

What kind of gains you think we achieve by that ? Could you please explain a bit more what is the INTENT here ?

[Andrushika]

Hi @zozo123, thanks for clarifying. Could you please add your "intent" to the PR description? This could help maintainers better understand what this PR is solving without reading through the comments.

[potiuk]

Thanks for splitting this out — the review is easier at this size.

Four of these changes are clear wins and I'd take them on their own: compression-level: '0' (the stash action does default to '6', so the tarball was being gzipped twice), the faster compressor, narrowing rm -rf ~/.cache to ~/.cache/prek so uv/pip caches survive for later steps in the job, and not re-uploading an unchanged payload on a hit.

Two changes appear to work against the stated goal:

1. The widened hashFiles glob. **/pyproject.toml matches 141 files in this repo, **/uv.lock 3, and **/.pre-commit-hooks.yaml matches none at all. But ~/.cache/prek holds hook environments built from .pre-commit-config.yaml, where every additional_dependencies is pinned inline. pyproject.toml appears in that config only in files: selectors — what a hook runs on, not where its environment comes from. The mypy hooks that do read uv.lock build into .build/mypy-venvs/, not the prek cache; and the only part of uv.lock that reaches prek is already in the key as UV_VERSION/PREK_VERSION. 18 of the last 332 commits on main touched one of those files, so this trades away warm caches for no correctness gain. I'd keep just **/.pre-commit-config.yaml.

2. Refresh policy vs. validation. The reason given for always running prek install-hooks is that an extracted archive can still be partial. But in exactly that case STASH_HIT and TAR_RESTORED are both true, so cache-policy sets save=false and the repair is discarded — the next run restores the same partial archive and repairs it again, indefinitely, except on schedule. Those two halves need to agree: if install-hooks changed anything, that should force a save.

On the absolute paths in the identity (python_executable, python_prefix, home, GITHUB_WORKSPACE) — the reasoning is right in principle, venvs do embed absolute paths. My concern is blast radius: save-cache: true is set in only two places, one writer per CI run feeding nine reader call sites. If the writer and the readers ever sit on different runner classes, this doesn't lower the hit rate, it zeroes it.

Which brings me back to my earlier question. Could you post before/after numbers — cache hit rate and the new "Prek cache preparation" timings over a handful of runs? With six behaviours changing at once and two of them pulling opposite ways, measurement is the only way to tell which ones earned their place.

Smaller things: the comment removed at the cache-key step documented why the key is built here rather than per-caller, and that callers must run it after Breeze sets up PATH — that constraint still holds and is now unrecorded. The docstrings in prek_cache_key.py are imperative rationale rather than descriptions. GITHUB_WORKSPACE is in REQUIRED_INPUTS but never set in the action's env: block. And the emoji/quoting churn is unrelated to the stated scope.

---

This review was drafted by an AI-assisted tool and confirmed by an Airflow maintainer. The findings below are observations, not blockers; an Airflow maintainer — a real person — will take the next look at the PR. If you think a finding is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Airflow handles maintainer review: contributing-docs/05_pull_requests.rst

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[potiuk]

This addresses both structural concerns from the last pass, and the way it does it is better than what I asked for.

The key is back to hashFiles('**/.pre-commit-config.yaml') alone, with prek${PREK_VERSION} added — which is the right extra input, since the hook environments are built by that binary. The refresh policy and the validation step now agree: cache-repaired forces a save, so a repaired PR cache is republished instead of being rebuilt on every run forever. The absolute-path identity is gone entirely, which removes the writer/reader blast-radius question rather than answering it. The cache-key step's comment about why the key is built there and after Breeze sets up PATH is restored, and GITHUB_WORKSPACE/REQUIRED_INPUTS went with the dropped file.

On measurement: the body now disclaims a performance claim rather than asserting one, and the step summary emits per-phase timings and a save reason, so the numbers can be collected from real runs after this lands. That answers the ask.

scripts/tests/ci/test_prek_cache_optimizations.py is a genuinely good test for a CI action — it extracts the actual step scripts from the YAML and runs them, so the policy matrix, the retry loop, the archive round-trip (permissions and symlinks included), the corrupt/wrong-directory archives and the unrelated-cache preservation are all exercised against the code that ships, not a restatement of it.

One finding inline, on the detection mechanism's failure mode; summarised here since it is the substance of this review.

Repair detection fails open when the markers aren't there (scripts/ci/prek_cache_markers.py:47)

snapshot_markers() returns {} for a cache with no hooks//repos/ marker files. The before and after snapshots then compare equal, so cache-changed=false and change-detection-uncertain=false, and on a pull-request run with a stash hit cache-policy picks save=false, reason=unchanged.

That is the correct behaviour for a healthy cache today, and the wrong one for a cache whose markers moved. .prek-hook.json and .prek-repo.json are prek internals, and this PR now puts prek${PREK_VERSION} in the cache key precisely because prek gets upgraded. A rename in a future prek makes both snapshots empty, repair detection silently turns off, and the discard-the-repair loop this PR exists to fix comes back — while the step summary reports Installation metadata changed: false, which reads as healthy.

The only check that the markers exist at all is test_real_prek_repair_and_reuse, which is gated behind RUN_PREK_INTEGRATION=1; nothing in the repo sets that, so CI never runs it. And test_restored_hooks_are_always_validated currently pins the fail-open: it asserts cache-changed == "false" and change-detection-uncertain == "false" for a cache that contains no markers whatsoever.

The uncertain path already exists and already routes to a save, so this fails safe with a couple of lines — if the cache directory has content but the snapshot came back empty, report uncertainty instead of "unchanged".

Smaller things

• cache-policy's installation-failed branch can't be reached in production: install-hooks has no continue-on-error, so a failure there ends the job before the policy step runs. The parametrised case that covers it is testing a state the action can't be in.
• Marker equality is a proxy for "was repaired". It holds on 0.5.2 — the integration test shows a rebuilt environment rewrites its marker — but it is worth knowing that a repair which leaves the marker byte-identical would read as unchanged.

---

This review was drafted by an AI-assisted tool and confirmed by an Airflow maintainer. The findings below are observations, not blockers; an Airflow maintainer — a real person — will take the next look at the PR. If you think a finding is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Airflow handles maintainer review: contributing-docs/05_pull_requests.rst

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[zozo123]

One more commit after the marker fix, and a note on why the guard is shaped the way it is.

The extra commit. installation-succeeded is written to $GITHUB_OUTPUT twice and read by nothing. Its only consumer was the installation-failed policy branch you flagged as unreachable, and the composite action declares no outputs: block, so no caller can read it either. Those four lines go with the branch. The retry loop's installation_succeeded shell variable is a different thing and stays.

On the guard. I checked it against the alternatives rather than assuming. The one worth recording: snapshotting the whole cache tree instead of prek's marker files would be layout-independent and would need no uncertainty path at all — which sounds strictly better. It isn't. prek.log lives inside ~/.cache/prek and is truncated and rewritten on every invocation. Measured against a real warm cache built from this repo's own config, two consecutive install-hooks runs that repaired nothing: the marker snapshot compares equal (correct), while a whole-tree path→size snapshot differs in exactly one entry, prek.log, 229191 → 67540 bytes. tar -C ~ -czf puts it in the archive, so every restored cache carries one. A whole-tree snapshot would report a change on every run and save on every PR run — the opposite of what this PR is for. Excluding prek.log would trade two known prek internals for one, with no gain.

Each conjunct is load-bearing under mutation: removing the guard fails 4 of the new cases, dropping any(cache_dir.iterdir()) fails 2, dropping cache_dir.is_dir() fails 3 — on a genuine cache miss the before-snapshot would raise FileNotFoundError, forcing uncertainty and destroying the cache-repaired signal on the most common path. One trap worth flagging for anyone tempted to shorten it: any(cache_dir.glob("*")) is not equivalent, because Path.glob swallows PermissionError and returns empty, which fails open on precisely the case this closes.

Local verification: 1392 passed / 1 skipped across scripts/tests/, the gated RUN_PREK_INTEGRATION=1 integration test passes, and prek static checks are clean on the changed files.

---

Drafted-by: Claude Code (Opus 5) (no human review before posting)

[Andrushika]

Title looks stale after the rebase. We can change it into something like “Republish repaired prek caches and keep unrelated host caches”.

[potiuk]

Hello @zozo123 - thank you for your contributions to Apache Airflow!

The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 6 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:

• Add Boat backend for sandbox tools #71725 - Add Boat backend for sandbox tools
• Republish repaired prek caches and keep unrelated host caches #73182 - Republish repaired prek caches and keep unrelated host caches
• Optimize UI and Playwright dependency caching #73183 - Optimize UI and Playwright dependency caching
• Optimize CI disk cleanup #73184 - Optimize CI disk cleanup

These pull requests stay open because maintainers are already engaged in them - they count towards your limit:

• Add Islo backend for sandbox tools #71672 - Add Islo backend for sandbox tools
• Add OpenSandbox backend for sandbox tools #71676 - Add OpenSandbox backend for sandbox tools

This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are.

What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or gh pr reopen <PR_NUMBER> --repo apache/airflow. Reopen the ones you are ready to follow through - keep them rebased, respond to review comments and fix failing checks.

While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack.

Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[github-actions]

Backport failed to create: v3-3-test. View the failure log Run details

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status	Branch	Result
❌	v3-3-test

You can attempt to backport this manually by running:

cherry_picker acaa1cb v3-3-test

This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.