internal/service/coordinator/static_registry.go (1)

62-74: Shadowing host and err variables causes unexpected behavior.

The if statement at line 62 declares new host, portStr, and err variables that shadow the function's return parameters. This means when net.SplitHostPort succeeds, the returned host from line 73 is the inner-scoped variable, but the function's named return host remains empty if you later fall through to error paths.

While this code path returns early on line 73 so it works correctly now, shadowing named returns is fragile and can cause subtle bugs if the function is refactored. Consider avoiding shadowing:

♻️ Suggested refactor to avoid shadowing

-	if host, portStr, err := net.SplitHostPort(addr); err == nil {
-		p, err := strconv.Atoi(portStr)
+	if h, portStr, splitErr := net.SplitHostPort(addr); splitErr == nil {
+		p, err := strconv.Atoi(portStr)
 		if err != nil {
 			return "", 0, fmt.Errorf("invalid port: %w", err)
 		}
 		if p <= 0 || p > 65535 {
 			return "", 0, fmt.Errorf("port must be between 1 and 65535")
 		}
-		if host == "" {
+		if h == "" {
 			return "", 0, fmt.Errorf("host cannot be empty")
 		}
-		return host, p, nil
+		return h, p, nil
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/coordinator/static_registry.go` around lines 62 - 74, The
code shadows the function's named return variables by using := in the
net.SplitHostPort call; avoid shadowing by declaring locals first (e.g., h,
portStr, splitErr := net.SplitHostPort(addr) or use regular assignment to an
existing err variable) and then assign the local host to the named return (host
= h) before returning, ensuring you check and convert portStr with strconv.Atoi
into p and keep the existing validations in the same block; update the block
around net.SplitHostPort in static_registry.go (the if that currently declares
host, portStr, err) to use non-shadowing locals or explicit assignments.

internal/service/coordinator/static_registry_test.go (1)

211-228: Good IPv6 test coverage; consider adding edge case tests.

The new test cases cover the main IPv6 scenarios well. Consider adding tests for:

• Malformed brackets: [ipv6 (missing closing bracket)
• Empty brackets: []
• IPv6 with zone ID: fe80::1%eth0 (the code handles this via zone stripping)

🧪 Optional additional test cases

{
    name:        "malformed bracket - missing close",
    addr:        "[fdbd:dc02:1a:e12::31",
    expectError: true,
    errorMsg:    "invalid address format",
},
{
    name:        "empty brackets",
    addr:        "[]",
    expectError: true,
    errorMsg:    "invalid address format",
},
{
    name:     "ipv6 with zone id",
    addr:     "fe80::1%eth0",
    wantHost: "fe80::1%eth0",
    wantPort: 50055,
},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/coordinator/static_registry_test.go` around lines 211 - 228,
Add three edge-case table-driven tests alongside the existing IPv6 cases in
static_registry_test.go: a "malformed bracket - missing close" case with addr
"[fdbd:dc02:1a:e12::31", expectError: true and errorMsg like "invalid address
format"; an "empty brackets" case with addr "[]", expectError: true and errorMsg
"invalid address format"; and an "ipv6 with zone id" case with addr
"fe80::1%eth0", wantHost "fe80::1%eth0" and wantPort 50055. Place these entries
in the same test cases slice used by the existing IPv6 tests (the entries with
wantHost/wantPort), and ensure the test logic checks expectError/errorMsg when
provided and otherwise validates wantHost and wantPort.

internal/service/coordinator/client.go (1)

396-396: Good improvement for IPv6 support.

Using net.JoinHostPort correctly handles IPv6 addresses by automatically adding brackets when needed (e.g., [::1]:50055). This pairs well with parseAddress in static_registry.go which stores the IPv6 host without brackets.

The fmt.Sprintf("%s:%d", ...) patterns in internal/cmd/coord.go (line 215) and internal/runtime/builtin/redis/client.go (line 88) will also break with IPv6 addresses and should use net.JoinHostPort for consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/service/coordinator/client.go` at line 396, Replace manual "%s:%d"
formatting with net.JoinHostPort to correctly handle IPv6 in all address
constructions: update the address assembly in the coordinator command (the code
that currently uses fmt.Sprintf("%s:%d", member.Host, member.Port) in the coord
command builder) to use net.JoinHostPort(member.Host,
strconv.Itoa(member.Port)), and do the same in the Redis client initialization
(the place that formats host and port for the Redis client, e.g., NewRedisClient
or the function that constructs the Redis address) so IPv6 hosts are bracketed
consistently with parseAddress/static_registry usage.