fix: harden forwarder listener lifecycle and port errors

[Luqueee]

Summary

• Track active listeners created by the forwarder and clean them up on shutdown or serve termination.
• Make live port additions return bind failures instead of reporting false success.
• Serialize duplicate checks and listener creation.
• Return HTTP 409 only for typed port conflicts and HTTP 500 for operational controller errors.
• Add regression coverage for listener lifecycle, bind failures, duplicate additions, and controller errors.

Fixes #18
Fixes #14

Validation

• go test ./...
• go test -race ./...
• go vet ./...
• CodeRabbit CLI review of the patch: 0 findings

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: e6bf3c1a-f5cb-4c13-8d71-0db2250e6635

📥 Commits

Reviewing files that changed from the base of the PR and between c2c9e60 and 13aff94.

📒 Files selected for processing (6)

• .coderabbit.yaml
• CODE_REVIEW.md
• cmd/mole/forwarder_test.go
• cmd/mole/main.go
• internal/admin/admin.go
• internal/admin/admin_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes

  • Improved SSH forwarding reliability during reconnects and listener cleanup.
  • Port conflicts are now reported clearly, while unexpected forwarding failures receive an appropriate server error.
  • Prevented stale listeners and duplicate port registrations from causing inconsistent forwarding behavior.

• Tests

  • Expanded coverage for listener registration, cleanup, bind failures, port conflicts, and controller error handling.

• Documentation

  • Added guidance for reviewing tunnel, discovery, configuration, CLI, and production deployment changes.

Walkthrough

The pull request updates CodeRabbit review policy and adds subsystem review contracts. It also tracks active forwarder listeners through creation, serving, replacement, and shutdown, and distinguishes port conflicts from controller failures in HTTP responses.

Changes

Review policy configuration

Layer / File(s)	Summary
Repository review controls .coderabbit.yaml, CODE_REVIEW.md	The repository now defines review controls, path-specific instructions, security checks, and subsystem correctness contracts.

Listener lifecycle and HTTP errors

Layer / File(s)	Summary
Atomic listener tracking and cleanup cmd/mole/main.go, cmd/mole/forwarder_test.go	Listener creation now returns bind errors, registers listeners before serving, removes only matching active entries after termination, and deletes entries during shutdown. Tests cover registration, cleanup, bind failures, and duplicate ports.
Port conflict and controller error mapping cmd/mole/main.go, internal/admin/admin.go, internal/admin/admin_test.go	AddDiscover returns admin.ErrConflict for excluded or duplicate ports and propagates operational errors. The admin endpoint maps conflicts to HTTP 409 and other failures to HTTP 500. Tests cover controller failures.

Merge Risk: ⚪ Minimal · up to 13aff

The change improves listener lifecycle cleanup and live port-addition error responses, with regression coverage for the updated behavior. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The implementation satisfies the main requirements for accurate live port errors in [#14]. It also registers and cleans up active listeners for [#18]. However, the provided test summary does not show …	Add regression tests that verify auto-discovered listeners appear in status, are pruned by retain, are removed by RemoveDiscover, and are cleaned up during shutdown. Alternatively, provide evidence that existing tests cover these requiremen…
Out of Scope Changes check	⚠️ Warning	The forwarder and admin changes match the linked issues. The .coderabbit.yaml and CODE_REVIEW.md additions are process and review-documentation changes that are not part of the linked listener lifecyc…	Remove the .coderabbit.yaml and CODE_REVIEW.md changes from this pull request, or link them to a separate issue that explicitly covers the review-configuration and documentation updates.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title uses the required Conventional Commit format and clearly describes the forwarder lifecycle and port-error changes.
Description check	✅ Passed	The description directly summarizes the listener lifecycle, live port error handling, HTTP status mapping, tests, and linked issues.

Full details: Linked Issues check

Explanation

The implementation satisfies the main requirements for accurate live port errors in [#14]. It also registers and cleans up active listeners for [#18]. However, the provided test summary does not show explicit regression coverage for status reporting, pruning through retain, or removal through RemoveDiscover, although [#18] requires coverage for these lifecycle behaviors.

Resolution

Add regression tests that verify auto-discovered listeners appear in status, are pruned by retain, are removed by RemoveDiscover, and are cleaned up during shutdown. Alternatively, provide evidence that existing tests cover these requirements.

Full details: Out of Scope Changes check

Explanation

The forwarder and admin changes match the linked issues. The .coderabbit.yaml and CODE_REVIEW.md additions are process and review-documentation changes that are not part of the linked listener lifecycle or HTTP error requirements.

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/harden-coderabbit

---

_{Comment @coderabbitai help to get the list of available commands.}