Skip to content

fix: purge stale .tfplan files when updating working dir to new ref#6358

Open
bakayolo wants to merge 1 commit intorunatlantis:mainfrom
bakayolo:fix/purge-stale-plans-on-ref-update
Open

fix: purge stale .tfplan files when updating working dir to new ref#6358
bakayolo wants to merge 1 commit intorunatlantis:mainfrom
bakayolo:fix/purge-stale-plans-on-ref-update

Conversation

@bakayolo
Copy link
Copy Markdown
Contributor

@bakayolo bakayolo commented Apr 2, 2026

What happened

When a new commit is pushed to a PR, updateToRef runs git reset --hard to move to the new ref. Since .tfplan files are untracked, git reset does not remove them. A subsequent bare atlantis apply discovers these stale plans via PendingPlanFinder and applies plans generated for a previous commit against the current infrastructure state.

This was introduced in #5895 ("Do not delete working dir if git falls behind"), which replaced the full reclone with git reset --hard + merge for performance. The full reclone naturally wiped all .tfplan files, but git reset --hard leaves untracked files intact.

Reproduction scenario

  1. Push commit A → plan runs → writes default.tfplan
  2. Push commit B → updateToRef does git reset --hard to commit B
  3. default.tfplan from commit A survives (untracked files are not cleaned by git reset --hard)
  4. User comments atlantis apply (bare) → PendingPlanFinder.Find() discovers the stale plan → applies it

Prior art

How I fixed it

Added a deletePlanFiles method to FileWorkspace that walks the clone directory and removes all .tfplan files (skipping .terragrunt-cache directories). This is called at the end of updateToRef for both the branch and merge checkout strategies, immediately after the ref is updated.

This is the narrowest fix — it catches the problem at the earliest possible point (when the commit changes), before any plan discovery runs.

Changes

  • server/events/working_dir.go:
    • Added deletePlanFiles() helper that uses filepath.Walk to find and delete .tfplan files
    • Call it at the end of both branch and merge paths in updateToRef()
  • server/events/working_dir_test.go:
    • Updated TestClone_ResetOnWrongCommit and TestClone_ResetOnWrongCommitWithMergeStrategy to assert that stale plan files are deleted (previously they asserted plan files survived — that was the buggy behavior)

Testing

All existing tests pass with the updated assertions. The fix was validated against both the branch strategy (TestClone_ResetOnWrongCommit) and merge strategy (TestClone_ResetOnWrongCommitWithMergeStrategy) code paths.

Copilot AI review requested due to automatic review settings April 2, 2026 20:13
@dosubot dosubot bot added bug Something isn't working go Pull requests that update Go code labels Apr 2, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a safety issue where untracked .tfplan files can survive git reset --hard during updateToRef, allowing a subsequent bare atlantis apply to apply a stale plan created for a previous commit.

Changes:

  • After updating the working directory ref (branch strategy and merge strategy), remove stale .tfplan files from the workspace.
  • Add deletePlanFiles() helper to walk the workspace and delete .tfplan files (skipping .terragrunt-cache).
  • Update clone/reset tests to assert that stale plan files are deleted after moving to a new commit.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
server/events/working_dir.go Deletes stale .tfplan files after updateToRef to prevent applying plans from a previous commit.
server/events/working_dir_test.go Updates reset-on-wrong-commit tests (branch + merge strategy) to assert .tfplan cleanup occurs.

@bakayolo bakayolo force-pushed the fix/purge-stale-plans-on-ref-update branch from c71302d to 7407ffb Compare April 2, 2026 20:39
When a new commit is pushed to a PR, updateToRef runs `git reset --hard`
to move to the new ref. Since .tfplan files are untracked, git reset does
not remove them. A subsequent bare `atlantis apply` would then discover
these stale plans via PendingPlanFinder and apply plans generated for a
previous commit against the current state.

Fix this by deleting all .tfplan files (excluding .terragrunt-cache dirs)
after every updateToRef, for both the branch and merge checkout strategies.
This ensures that a bare apply can never pick up plans from a previous
commit.

Co-authored-by: Amp <amp@ampcode.com>
Signed-off-by: Ben Apprederisse <bena@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d4fcc-fbba-7678-92bd-d1cfa957e5cb
@bakayolo bakayolo force-pushed the fix/purge-stale-plans-on-ref-update branch from 7407ffb to c435213 Compare April 2, 2026 20:49
Copy link
Copy Markdown

@nandiheath nandiheath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't mean to approve the PR but doesn't seem like i can dismiss it :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants