[shin0343]

A node can hold three routers while configured for two: the monitor attaches a spare while a dropped router is being redialed, then the old router returns and its existing attachment becomes live again. A top-up that finishes after another path filled the target can overflow it too.

Make attachment admission and connection-manager protection one n.mu decision. Keep existing live attachments, refuse a late surplus attachment, and release disconnected attachment intents once replacements fill the target. Pending redials then stop at their existing attachment check; an already-running handshake may finish without restoring the retired attachment. Keep retrying dropped routers while the node is still short, and refresh AutoRelay candidates after retirement. Log both retirement and surplus admission decisions.

The dropped set and live count come from the same snapshot, so a router that was live and drops later retains its own retry intent. Shared connections can carry transit traffic, so retirement removes the held-router/protection state without forcibly closing every connection. No dependencies, flags, or protocol changes.

Related to #277, specifically the SIGKILL-and-return overflow reproduction. This proposes retaining the healthy replacement rather than switching back after recovery.

Validation on Linux/arm64, Go 1.27.3:

• All three new regression tests fail on unchanged main (3669094): in-flight redial overflow, late/concurrent replacement admission, and retirement only once the target is full. They also check connection protection and AutoRelay candidates.
• Focused router selection, startup, reconnect, and new regression tests pass with -race.
• Concurrency regression tests: 200 stress runs, eight parallel processes restricted to two CPUs, zero failures. Tests synchronize on observed state and handshake channels; no added sleeps or widened timeouts.
• make and make test: pass; 837 top-level tests passed, 9 existing JS/Python SDK integration tests skipped because their toolchains are absent.
• make lint: pass (formatting, both Helm charts, pinned golangci-lint v2.14.0 with zero issues, and deadcode).
• make e2e-test: all 32 Bats scenarios pass. An initial setup attempt exhausted the validation VM disk while importing kind images; reclaiming the anonymous volumes left by earlier test clusters resolved that environment failure. No test assertions changed.

Updated the networking explanation to distinguish held attachments from transient router connections.

AI assistance: implemented with Codex and checked through independent read-only Claude code/documentation reviews.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request refactors the router attachment logic to prevent exceeding the target router limit during concurrent top-ups or redials. It introduces atomic checks under a single lock and automatically releases dropped routers once replacements fill the target. Documentation is updated to reflect these changes, and new tests are added. However, a compilation error was found in the test file where sync.WaitGroup is incorrectly used with a non-existent Go method.

[gemini-code-assist]

The standard library sync.WaitGroup does not have a Go method. This will cause a compilation error. Use attaching.Add(1), defer attaching.Done(), and a standard go statement instead.

Suggested change

	var attaching sync.WaitGroup
	for _, id := range []peer.ID{one.h.ID(), two.h.ID()} {
	attaching.Go(func() {
	<-ready
	node.attach(id)
	})
	}
	var attaching sync.WaitGroup
	for _, id := range []peer.ID{one.h.ID(), two.h.ID()} {
	attaching.Add(1)
	go func(id peer.ID) {
	defer attaching.Done()
	<-ready
	node.attach(id)
	}(id)
	}

[shin0343]

sync.WaitGroup.Go was added in Go 1.25: https://pkg.go.dev/sync#WaitGroup.Go. This repository requires Go 1.26.6 in go.mod, so this method is available on its supported toolchain. I also reran all three router-replacement regression tests under -race with Go 1.27.3; they compile and pass. Keeping the standard-library helper here.

[aojea]

@shin0343 will the current behavior not be eventual consistent? or do you mean it will keep attached to many routers?

[shin0343]

@aojea Yes, it can keep the extra held router indefinitely while all three stay healthy; it does not converge back to the configured target on its own.

For a target of 2:

1. The node holds A and B.
2. A disconnects; its attachment/protection is retained while its redial retries.
3. The monitor sees only one live attachment and attaches C.
4. A returns before its retries are exhausted. Its successful redial makes the retained attachment live again, leaving A, B and C held and protected.

On current main, topUpRouters returns immediately when have >= want; there is no corresponding surplus-retirement path. A successful redial also returns without detaching the recovered router. The connection manager cannot trim these connections because all held routers have the router protection tag.

I rechecked this against today's main (1fae868e) with TestRouterReplacementDuringRedial: after A's redial completed, three further calls to topUpRouters each still reported 3 live held routers, target 2. The same regression on this PR passes with B and C retained; the concurrent-admission and retry-until-full tests also pass under -race.

The fix retires A's attachment/protection when B+C fill the target, so its in-flight handshake cannot restore a third hold. A transport connection may still exist for other mesh traffic; the invariant here is the number of held router attachments, not every open libp2p connection.

Separately, the remaining Android CI failure occurs while installing the emulator (Error reading Zip content from a SeekableByteChannel), before the test script runs. I tried rerunning the failed job, but GitHub requires repository admin rights. Could you rerun that failed job?