[test-improver] Improve tests for server/circuit_breaker

[github-actions]

File Analyzed

• Test File: internal/server/circuit_breaker_test.go
• Package: internal/server
• Lines Added: +129

Improvements Made

1. Better Testing Patterns

• ✅ All new tests use table-driven structure with t.Parallel() subtests
• ✅ Descriptive test names following TestType_Scenario convention
• ✅ Uses require for fatal preconditions, assert for non-fatal checks

2. Increased Coverage

Four previously untested code paths now have direct tests:

New Test	Code Path Covered
TestCircuitBreakerState_String	String() method for all 4 states including defensive default: "UNKNOWN" case
TestFormatResetAt	formatResetAt() helper — zero time and non-zero RFC3339+duration output
TestIsRateLimitText_Direct	isRateLimitText() — all 5 match patterns plus edge cases (case insensitivity, empty string, "rate limit" without qualifier)
TestCircuitBreaker_RecordRateLimitWhenAlreadyOpen	RecordRateLimit() OPEN→OPEN path — verifies resetAt is updated on subsequent errors while circuit stays OPEN

Previously these were either:

• Never tested: String() default case, formatResetAt() directly
• Only tested indirectly: isRateLimitText() via isRateLimitToolResult(), the OPEN→OPEN path via logging only

3. No Regressions

• All existing tests are preserved unchanged
• No test helpers or fixtures modified

Why These Changes?

The circuit breaker is security-critical infrastructure (rate-limit protection). The four gaps found were:

1. The String() method's default case is defensive code that can never occur with valid iota values — but it should be documented via a test so future readers know it exists and won't get dropped in refactoring.
2. formatResetAt is called in ErrCircuitOpen.Error() and several log lines but had no isolated test — a regression in its formatting would only surface through integration test failures.
3. isRateLimitText has 5 OR-combined patterns. The indirect tests covered only 4 of them through isRateLimitToolResult. The "rate limit combined with 403" branch (strings.Contains(lower, "rate limit") && strings.Contains(lower, "403")) had no test for a string that contains "rate limit" without "403" being present — confirming the boundary between true/false is correctly placed.
4. The OPEN→OPEN RecordRateLimit path (updating resetAt while already open) is a normal operational scenario (multiple rate-limit errors in a row) that had no test at all.

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Generated by Test Improver · ● 14.7M · ◷

[Copilot]

Pull request overview

This PR strengthens internal/server’s circuit breaker unit test coverage by adding direct, table-driven tests for previously untested helper/state paths, improving confidence in rate-limit detection and circuit state handling.

Changes:

• Added direct unit tests for circuitBreakerState.String() including the defensive unknown-state case.
• Added isolated tests for formatResetAt() (zero vs non-zero time formatting).
• Added direct pattern/edge-case coverage for isRateLimitText() and the OPEN→OPEN RecordRateLimit() path (reset time update while staying OPEN).

Show a summary per file

File	Description
internal/server/circuit_breaker_test.go	Adds targeted table-driven tests covering helper functions and an additional circuit breaker state transition path.

Copilot's findings

Tip

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

• Files reviewed: 1/1 changed files
• Comments generated: 0