Recover GitDagBundle from a clone interrupted by a killed process - #72787

Thanks for picking this up. One bit of context that may be useful before this is merged: #72759 was opened after noticing that #71535 appears to recover the same interrupted-clone state through a narrower path, and I already offered an explicit regression test for that case on #71535.

The two approaches seem to differ mainly in recovery scope:

• Fetch missing Git bundle tracking refs #71535 checks whether tracking_ref is resolvable and, only when it is not, fetches from the version clone's local bare origin before checkout. For the state in GitDagBundle: an interrupted clone leaves a version directory that can never check out and is never repaired #72759, that should repopulate the missing refs without recreating the working clone.
• This PR moves checkout into _clone_repo_if_required()'s existing retry/cleanup block, so any GitCommandError during checkout removes the working clone and retries from the healthy bare mirror. That also fixes GitDagBundle: promoting a SHA-pinned tracking_ref can fail with 'reference is not a tree' if local storage isn't cleared #71388, but is a broader behavior change because checkout failures that are not caused by an interrupted clone will also trigger cleanup/reclone.

The regression test added here is exactly the kind of coverage I was hoping to preserve for #72759, so I think the test itself is valuable either way.

Would a maintainer mind taking a quick look at #71535 and this PR together and deciding which recovery behavior is preferable? If the broader cleanup/reclone semantics are intentional, this PR covers #72759 directly. If the narrower fetch-before-checkout behavior is preferred, the interrupted-clone regression test could instead be added to #71535 so this failure mode remains explicitly covered.

Thanks for the context. I agree the narrower fetch-before-checkout path in #71535 may be preferable if we want to limit the recovery scope. I’ll leave the implementation unchanged for now and let the maintainers compare #71535 and #72787. The interrupted-clone regression test can be moved to #71535 if that approach is preferred.

This fixes a second leftover state that we hit in production, which is worth recording here
because it is the case where the two competing approaches diverge.

Airflow 3.3.0, providers-git 0.4.x, long-lived worker pods with pod-local bundle storage. A task
was marked failed from the UI while its worker was cloning the tracking repo; SIGTERM landed 315
ms into the clone. git clone had written files into the working tree but had not recorded them
in the index, so the checkout failed on untracked files rather than on a missing ref:

git checkout master
error: The following untracked working tree files would be overwritten by checkout:
        .agents/instructions/airflow-patterns.md
        <...>
Aborting

All 30 tasks that landed on that pod over the next 23 minutes failed at
task_runner.py parse -> git.py:244 initialize -> git.py:215 _initialize. Sibling pods in
the same ReplicaSet were fine, one completed 1,180 tasks the same day, and it was the only worker
in three weeks with a 100% failure rate. It only cleared when the workers rolled to a new
ReplicaSet.

Your change recovers this: the GitCommandError reaches the existing
except (InvalidGitRepositoryError, GitCommandError) handler, rmtree(repo_path) runs, and the
retry produces a clean clone. #71535 does not, because fetching the missing refs does nothing
about untracked files blocking the checkout. So the broader recovery here is not just a
stylistic preference over the narrow fetch, it covers a state the fetch cannot reach.

Happy to contribute a regression test for it if you want one in this PR: clone with
--no-checkout, copy the tracked files into the working tree so they are untracked, then assert
initialize() recovers. It fails on main for a different reason than your existing test, so the
two are not redundant.

Separately I have opened #72922 and a PR for the prevention half, staging the clone in a sibling
directory and renaming it into place so a killed clone cannot leave anything at repo_path at
all. It is deliberately not a competing fix for the recovery, and the two compose. We both touch
the clone block, so whoever merges second will need a trivial rebase.

@hkc-8010
Thanks, that would be helpful. I agree this production state is worth covering with a regression test. Please feel free to add the test to this PR

Hello @Kunal8954 - 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 8 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:

• Log blocking run/task IDs when a DAG hits max_active_runs #72403 - Log blocking run/task IDs when a DAG hits max_active_runs
• Decouple workload JWT token lifetime from task_queued_timeout #72488 - Decouple workload JWT token lifetime from task_queued_timeout
• Preserve naive datetime XCom values across worker timezones #72641 - Preserve naive datetime XCom values across worker timezones
• Recover GitDagBundle from a clone interrupted by a killed process #72787 - Recover GitDagBundle from a clone interrupted by a killed process
• Clear the task state store when clearing a task instance #72788 - Clear the task state store when clearing a task instance
• Add Stats.observable_gauge for callback-based OTel gauges #72887 - Add Stats.observable_gauge for callback-based OTel gauges

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

• Amazon: Support managed Bedrock knowledge bases #72608 - Amazon: Support managed Bedrock knowledge bases
• Keep legacy Dag tree links working #72644 - Keep legacy Dag tree links working

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