ntf: stop a managed server's fatal from exiting the service

[ripienaar]

Managed server ports now come from a configurable range below the OS ephemeral range, and a create that loses a port to another program is retried with fresh ports.

Each managed server logs to its own file, and that logger's Fatalf calls os.Exit, so one managed server failing to bind a port killed the whole service with nothing on its console. Managed servers now log through a wrapper whose Fatalf writes to server.log and the service log, records the error and returns, so the create fails and the service stays up.

[synadia-claude-reviewer]

Claude finished @ripienaar's task in 2m 55s —— View job

---

Review of PR #87 complete

• Fetch PR diff and metadata
• Review changed files for correctness, security, performance
• Check goroutine/channel/context handling, mutexes, error handling
• Post review summary

Summary: Traced the retry/rollback and port-reservation logic end to end (ntf/handlers.go, ntf/ports.go, ntf/serverlog.go, ntf/service.go). Port accounting under rollback and concurrent teardown is correct, retries can't cause a double response, and the fatalGuard/waitForStart mechanism correctly stops a managed server's Fatalf from exiting the process while still catching late fatals after readiness. No security or correctness blockers.

Left two non-blocking observations as inline comments and a summary comment:

1. ports.go — portAllocator.reserve() holds its mutex for the whole scan; potential latency under heavy concurrent load, not a correctness issue.
2. serverlog.go — classifying a retryable listen failure depends on an internal detail of how the pinned nats-server version calls Fatalf; currently covered by TestCreateFailsWhenPortStaysTaken but could silently regress on a future nats-server bump.

[synadia-claude-reviewer]

Review summary

Traced the retry/rollback and port-reservation paths (ntf/handlers.go, ntf/ports.go, ntf/serverlog.go, ntf/service.go) end to end:

• Port accounting is correct under rollback and concurrent teardown. reservePort appends to inst.ports under s.mu and re-checks the instance is still registered before doing so, so a reservePort racing a concurrent destroy/reset can't leak a port that tearDownInstance already stopped tracking. Every retry path (createServerAttempt/createClusterAttempt/createSuperClusterAttempt) rolls back via dropInstance → tearDownInstance, which shuts down whatever servers already started in that attempt and releases the full inst.ports set, so a mid-cluster failure doesn't leak nodes or ports.
• retryCreate can't double-respond. Retry (return true) only happens immediately after rollback(), before any req.Error/req.RespondJSON call, and the final attempt always forces retryAfter to return false, so the request is guaranteed exactly one response.
• fatalGuard/waitForStart correctly prevents nats-server's Fatalf-triggered os.Exit from taking down the service, and checks the guard's recorded error after ReadyForConnections on every poll, so a late-arriving fatal after readiness is still caught.

Two non-blocking points left as inline comments:

1. ports.go: portAllocator.reserve() holds its mutex for the whole scan (up to the full range size in blocking syscalls) — a potential latency cliff under concurrent load with a heavily-subscribed range, not a correctness issue.
2. serverlog.go: classifying a fatal as a retryable listen failure depends on nats-server passing a *net.OpError{Op:"listen"} into Fatalf's args — an internal detail of the pinned dependency version. It's exercised by TestCreateFailsWhenPortStaysTaken today, but a future nats-server bump could silently regress the retry path with no obvious signal.

No security or correctness blockers found.

[piotrpio]

LGTM!