fix: persist plan results before publishing success - #6848
Conversation
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall line coverage in commit 72d59f4 in the Show a line coverage summary of the most covered files.
Updated |
There was a problem hiding this comment.
🟢 Approval recommended
The ordering change is well-scoped and backed by deterministic tests; only a minor logging-nit was identified.
Pull request overview
This PR tightens Atlantis’s plan/autoplan lifecycle by ensuring PullStatus is durably persisted before publishing any successful plan output (success comment, per-project plan checks, and aggregate plan/apply checks), and by failing loudly/explicitly when persistence fails to avoid misleading “success” signals.
Changes:
- Defers successful per-project plan status updates until after the DB write boundary (while still emitting failed statuses immediately).
- Reorders plan/autoplan publication so the success comment and aggregate statuses are only published after PullStatus persistence succeeds; on persistence failure, publishes an actionable error and marks plan/apply aggregates as failed.
- Adds deterministic regression tests covering ordering for manual plan/autoplan and persistence-write failures via production wrappers.
File summaries
| File | Description |
|---|---|
| server/events/project_command_runner.go | Adds deferred plan-status publishing and defers non-API plan success project checks until after persistence. |
| server/events/plan_command_runner.go | Reorders plan/autoplan publication to persist first; adds persistence-failure handler and deferred plan-status publishing hook. |
| server/events/instrumented_project_command_runner.go | Passes through the new deferred plan-status publisher interface for instrumented runners. |
| server/events/command_runner_test.go | Allows tests to wrap the plan runner independently (to observe/verify persistence boundary behavior). |
| server/events/plan_command_runner_test.go | Adds deterministic tests asserting persistence happens before success publication and that write failures fail plan/apply aggregates with actionable messaging. |
| server/events/project_command_runner_test.go | Updates expectations so plan success project checks are deferred in non-API flows (and apply success remains deferred). |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses #6641 Assisted-by: OpenAI GPT-6 <noreply@openai.com> Signed-off-by: Rui Chen <rui@chenrui.dev>
Assisted-by: OpenAI GPT-6 <noreply@openai.com> Signed-off-by: Rui Chen <rui@chenrui.dev>
7c1e948 to
72d59f4
Compare
Summary
Persist plan/autoplan PullStatus before publishing the success comment or successful per-project and aggregate checks. If persistence fails, publish an actionable error and fail the plan/apply aggregates instead of leaving a misleading success.
Addresses #6641.
Notes
Extracts the persistence-ordering invariant from #6657, without generations, publication claims, or plan-store changes. Existing empty/targeted-plan and cleanup behavior is preserved. #6781 is already on the base and is not duplicated.
Deterministic tests cover manual/autoplan ordering and write failures through the production output/metrics wrappers. Existing targeted-empty-plan status-preservation tests remain in place. Local lint reports three unchanged findings (two modernize suggestions and one deprecated GitLab test field); hosted checks are still required.