feat: Feature Status manager #5763
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cts found This fixes issue runatlantis#5389 where PRs were getting stuck with pending status when ATLANTIS_SILENCE_VCS_STATUS_NO_PLANS and ATLANTIS_SILENCE_VCS_STATUS_NO_PROJECTS were enabled and no projects matched when_modified patterns. Root cause: - PR runatlantis#5242 (commit be06063) introduced early pending status setting in command_runner.go - When silence flags are enabled and no projects are found, the pending status was never cleared - This left PRs stuck in pending state, blocking auto-merge functionality Solution: - Modified plan_command_runner.go to clear pending status even when silence flags are enabled - Added else blocks to both autoplan and manual plan paths - When silence is enabled but no projects found, update status to success (0/0) to clear pending - Added comprehensive test to prevent regression Testing: - Updated existing test expectation for silence flag behavior - Added new test TestPlanCommandRunner_SilenceFlagsClearsPendingStatus - All existing tests continue to pass
Similar to plan_command_runner.go, when no projects are found and silence flags are enabled, clear pending status to avoid leaving PR checks stuck in pending state. Addresses review feedback in PR runatlantis#5713.
- Check for no projects before setting pending status in API controller - When silence flags are enabled and no projects found, don't set any VCS status - This results in no status checks appearing instead of success with 0/0 projects - Addresses feedback that silence should mean no status checks at all
- Add SilenceVCSStatusNoProjects flag to DefaultCommandRunner - Only set pending status in command_runner.go when silence is not enabled - This prevents any status checks from being created when silence is enabled - Works for both autoplan and manual commands
…dling - Create status package with StatusManager, StatusPolicy, and StatusCleaner - Use Go idiomatic dependency inversion (interfaces where consumed) - Replace scattered silence flag logic with centralized policy decisions - Maintain full backward compatibility with existing silence flags - No new configuration flags needed - Clean separation of concerns and easier testing Architecture benefits: - Single responsibility: all status logic in one package - Policy-driven: centralized silence flag handling - Testable: easy to mock and test status behavior - Maintainable: changes only need to be made in one place - Extensible: easy to add new status types or policies - Clear: explicit status clearing operations available
…anup - Implement fork PR detection in shouldSilenceForkPR() using existing logic - Document GetCurrentStatus() method for future VCS status querying implementation - Remove redundant CleanupPendingOnly() method - use ClearPendingStatuses() directly 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…s updates This comprehensive implementation replaces scattered status update logic across command runners with a centralized StatusManager architecture that provides policy-driven decisions about when to set, clear, or silence VCS status checks. ## Key Features ### 🏗️ Architecture - **StatusManager**: Central orchestrator for all VCS status decisions - **StatusPolicy**: Encapsulates silence flag logic and fork PR detection - **Clean Interface**: Command runners use semantic methods instead of direct status calls ### 🔧 Command Runner Integration - **ApplyCommandRunner**: Now uses StatusManager.HandleNoProjectsFound(), SetFailure(), SetSuccess() - **PolicyCheckCommandRunner**: Integrated with StatusManager for all status decisions - **ApprovePoliciesCommandRunner**: Added StatusManager integration - **DefaultCommandRunner**: Enhanced with StatusManager.HandleCommandStart() - **PlanCommandRunner**: Already properly integrated ### 🎯 Silence Flag Handling - **Complete Coverage**: All silence flags now work consistently across command types - **Fork PR Detection**: Uses ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner - **No Status When Silenced**: When silence flags are enabled, NO VCS status is set (not even success 0/0) - **Backward Compatible**: No new flags needed, existing configuration works seamlessly ### 🔍 Issues Resolved - ✅ **PR runatlantis#5713**: VCS status no longer gets stuck in pending state when silence flags are enabled - ✅ **Consistency**: All command types now respect silence flags uniformly - ✅ **Fork PRs**: Proper handling of fork PR status updates with silence support - ✅ **Maintainability**: Single source of truth eliminates duplicate status logic ## Implementation Details ### Status Decision Flow 1. Command event occurs (start/end/no projects) 2. StatusManager delegates to StatusPolicy 3. Policy evaluates silence flags and fork PR status 4. Returns StatusDecision (Set/Clear/Silence) 5. StatusManager executes decision or skips VCS interaction ### Silence Flag Combinations - `SilenceNoProjects`: Controls PR comments - `SilenceVCSStatusNoPlans`: Controls plan command status when no projects found - `SilenceVCSStatusNoProjects`: Controls all command status when no projects found - `SilenceForkPRErrors`: Controls all status updates for fork PRs ### Files Modified - `server/events/apply_command_runner.go`: StatusManager integration - `server/events/policy_check_command_runner.go`: StatusManager integration - `server/events/approve_policies_command_runner.go`: StatusManager integration - `server/server.go`: StatusManager construction and injection - `server/events/command_runner_test.go`: Updated test expectations - `server/events/plan_command_runner_test.go`: Fixed silence behavior tests - `server/events/mocks/mock_status_manager.go`: Manual mock for testing - `docs/status-manager.md`: Comprehensive architecture documentation ## Testing - ✅ All StatusManager unit tests pass - ✅ Updated command runner tests to match new behavior - ✅ Verified silence flags work correctly (no status when enabled) - ✅ Fork PR detection working properly - ✅ Backward compatibility maintained ## Documentation - **Architecture Guide**: Complete StatusManager documentation with diagrams - **Configuration Reference**: All silence flag combinations explained - **Migration Guide**: How status logic changed from scattered to centralized - **Troubleshooting**: Common issues and debugging techniques - **API Reference**: Full interface documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Move status-manager.md from docs/ to runatlantis.io/docs/ so it gets built for the website. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
jamengual
force-pushed
the
feature/status-manager
branch
from
638d57d to
5a4204f
Compare
jamengual
force-pushed
the
feature/status-manager
branch
from
5de79cb to
ac9d896
Compare
…owHook tests - Add StatusManager mock setup to simulate UpdateCombined call through HandleCommandStart - Remove unnecessary type arguments in Eq() calls to fix linter warnings - Both FailOnPreWorkflowHookError_False and FailOnPreWorkflowHookError_True tests now pass The tests were failing because ApplyCommandRunner now uses StatusManager instead of directly calling commitStatusUpdater. The mock setup ensures the expected behavior is maintained during testing.
Applied gofmt -s formatting to fix linting issues in: - server/events/apply_command_runner.go - server/events/apply_command_runner_test.go - server/events/command_runner.go - server/events/command_runner_internal_test.go - server/events/command_runner_test.go - server/events/mocks/mock_status_manager.go - server/events/policy_check_command_runner.go
- Handle error return value from commitUpdater.UpdateCombined in test mocks - Fix formatting in events_controller_e2e_test.go
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a comprehensive StatusManager architecture that centralizes all VCS status check decisions in Atlantis. The StatusManager replaces scattered status update logic throughout command runners with a single, policy-driven system.
Key Changes
Problem Solved
Fixes issue where VCS status checks would get stuck in "pending" state when silence flags were enabled. Previously,
command_runner.gowould set pending status but the status would never be cleared when silence flags prevented the final status update.Architecture
Status Decision Logic
The StatusManager now handles all status decisions through a centralized policy:
ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.OwnerFiles Modified
server/events/status/- New status management packageserver/events/*_command_runner.go- Updated all command runnersserver/server.go- StatusManager dependency injectionserver/events/mocks/- Updated mocks for new interfacesSilence Flag Behavior
When silence flags are enabled, no status is set at all (not success 0/0):
SilenceForkPRErrorsSilenceVCSStatusNoPlansSilenceVCSStatusNoProjectsDocumentation
Added comprehensive documentation at
runatlantis.io/docs/status-manager.mdcovering:Test Plan