Skip to content

Conversation

@ncode
Copy link
Owner

@ncode ncode commented Dec 25, 2025

Critical fixes:

  • Run elections concurrently in goroutines instead of blocking sequentially
  • Inherit parent environment in command execution (os.Environ)
  • Clone slices before modification to prevent data corruption

High priority fixes:

  • Add empty command check to prevent panic in runCommand
  • Filter health checks by service instance ID and ServiceChecks config
  • Add graceful shutdown with signal handling (SIGINT/SIGTERM)
  • Fix session renewal goroutine leak with cancellable context
  • Run first election immediately on startup instead of waiting TTL/2

Medium priority fixes:

  • Add Key field validation in constructor
  • Replace golang.org/x/exp/slices with stdlib slices package
  • Update CLI description and remove unused toggle flag

🤖 Generated with Claude Code

Critical fixes:
- Run elections concurrently in goroutines instead of blocking sequentially
- Inherit parent environment in command execution (os.Environ)
- Clone slices before modification to prevent data corruption

High priority fixes:
- Add empty command check to prevent panic in runCommand
- Filter health checks by service instance ID and ServiceChecks config
- Add graceful shutdown with signal handling (SIGINT/SIGTERM)
- Fix session renewal goroutine leak with cancellable context
- Run first election immediately on startup instead of waiting TTL/2

Medium priority fixes:
- Add Key field validation in constructor
- Replace golang.org/x/exp/slices with stdlib slices package
- Update CLI description and remove unused toggle flag

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@codecov
Copy link

codecov bot commented Dec 25, 2025

Codecov Report

❌ Patch coverage is 46.80851% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.35%. Comparing base (43d3164) to head (e7b7f80).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
cmd/run.go 0.00% 20 Missing ⚠️
internal/ballot/ballot.go 81.48% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
+ Coverage   53.07%   57.35%   +4.27%     
==========================================
  Files           4        4              
  Lines         552      469      -83     
==========================================
- Hits          293      269      -24     
+ Misses        227      168      -59     
  Partials       32       32              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add tests covering:
- New() validation for missing key and id fields
- runCommand() empty command error handling
- handleServiceCriticalState() filtering by ServiceID and ServiceChecks
- session() renewal cancellation when creating new sessions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link

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

This pull request addresses multiple bugs and reliability improvements in the ballot leader election system. The changes include critical fixes for concurrent execution, environment inheritance, data corruption prevention, and resource leak fixes.

Key Changes:

  • Replaces sequential election blocking with concurrent goroutines for multiple services
  • Fixes session renewal goroutine leak by adding cancellable context management
  • Prevents slice mutation issues by cloning before modification throughout the codebase

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
go.mod Updates Go version to 1.25.5 (invalid version - see comments) and updates dependency versions including consul/api, viper, and cobra
go.sum Corresponding checksum updates for dependency changes
internal/ballot/ballot.go Core fixes: adds Key validation, replaces experimental slices with stdlib, inherits os.Environ, validates empty commands, clones slices before mutation, filters health checks by service ID and ServiceChecks config, fixes session renewal leak with cancellable context, runs first election immediately
internal/ballot/ballot_test.go Adds comprehensive test coverage for Key/ID validation, empty command handling, session renewal cancellation, and health check filtering
cmd/run.go Implements concurrent election execution with goroutines, graceful shutdown via signal.NotifyContext, and proper synchronization with sync.WaitGroup
cmd/root.go Updates CLI description to accurately describe the tool's purpose and removes unused toggle flag
.github/workflows/go.yml Updates actions versions and Go version to 1.25 (invalid - see comments)
.github/workflows/ci.yml Updates actions versions, Go version to 1.25 (invalid - see comments), and adds GOTOOLCHAIN override
.github/workflows/codeql.yml Removes CodeQL workflow entirely

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ncode ncode merged commit 04bf30f into main Dec 25, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant