From b99d02b7a903260ae8c32f35068f4d8e4dfb3ac2 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 5 Jul 2026 22:43:22 +0200 Subject: [PATCH] fix(felhomsshd): keep the persisted port unconditionally (no self-listen flip-flop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a continuous reconcile felhom-sshd itself listens on the claimed port, so re-probing isFree(persisted) found it 'busy' by our own daemon and thrashed to another candidate every tick. A persisted port is ours — keep it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- internal/felhomsshd/claim.go | 17 ++++++++++------- internal/felhomsshd/felhomsshd_test.go | 12 ++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/internal/felhomsshd/claim.go b/internal/felhomsshd/claim.go index 884193c..3605c77 100644 --- a/internal/felhomsshd/claim.go +++ b/internal/felhomsshd/claim.go @@ -14,15 +14,18 @@ var ErrPortsExhausted = fmt.Errorf("felhomsshd: all candidate OOB ports are busy // for tests; production impl = probeFree (ss + net.Listen). type portProbe func(port int) bool -// claimPort returns the OOB port, mirroring the spike's shell algorithm: -// - if a persisted port exists AND is still free → keep it (idempotent, no thrash), -// - else the FIRST free candidate → persist + return, -// - else ErrPortsExhausted (LOUD). +// claimPort returns the OOB port: +// - if a port is already PERSISTED (and != 22) → keep it unconditionally. It is OUR port; on a +// continuous reconcile felhom-sshd is itself LISTENING on it, so re-probing with isFree would +// (wrongly) find it "busy" by our own daemon and thrash to another candidate every tick. Once +// claimed, the port is stable (the belt @ssh_port and the operator's known port depend on it). +// - else the FIRST free candidate → persist + return (isFree = ss-empty AND a real bind succeeds). +// - else ErrPortsExhausted (LOUD — never :22 or a random port). // -// persist writes PortFile; readPersisted reads it. isFree is the probe. All injected for tests. +// persist writes the port file; readPersisted reads it. isFree is the probe. All injected for tests. func claimPort(candidates []int, isFree portProbe, readPersisted func() (int, bool), persist func(int) error) (int, error) { - if cur, ok := readPersisted(); ok && cur != 22 && isFree(cur) { - return cur, nil + if cur, ok := readPersisted(); ok && cur != 22 { + return cur, nil // persisted = ours; keep it (no thrash — felhom-sshd holds it) } for _, p := range candidates { if p == 22 { diff --git a/internal/felhomsshd/felhomsshd_test.go b/internal/felhomsshd/felhomsshd_test.go index 2e0ac63..ebd7cfb 100644 --- a/internal/felhomsshd/felhomsshd_test.go +++ b/internal/felhomsshd/felhomsshd_test.go @@ -78,10 +78,11 @@ func TestClaimPort_CleanContentionIdempotentExhaustion(t *testing.T) { t.Fatalf("contention claim = %d / %v, want 2222", p, err) } - // idempotent: persisted 2222 still free (even though 8822 is now free) → keep 2222 (no thrash) - isFree, read, write, _ = claimHarness(map[int]bool{8822: true, 2222: true, 8022: true, 62222: true}, 2222) + // idempotent: a persisted port is KEPT unconditionally (even when isFree(cur) reports busy — + // that's felhom-sshd itself holding it on a continuous reconcile; re-probing would flip-flop). + isFree, read, write, _ = claimHarness(map[int]bool{8822: true, 2222: false, 8022: true, 62222: true}, 2222) if p, err := claimPort(cands, isFree, read, write); err != nil || p != 2222 { - t.Fatalf("idempotent claim = %d / %v, want 2222 (kept)", p, err) + t.Fatalf("persisted port must be kept even when isFree says busy (own daemon), got %d / %v", p, err) } // exhaustion: all busy → LOUD error, no fallback @@ -97,9 +98,4 @@ func TestClaimPort_CleanContentionIdempotentExhaustion(t *testing.T) { t.Fatalf("exhaustion must persist nothing, stored=%d", *stored) } - // persisted port that is now BUSY → re-claim a fresh free one (not the stale persisted) - isFree, read, write, _ = claimHarness(map[int]bool{8822: false, 2222: true, 8022: true, 62222: true}, 8822) - if p, err := claimPort(cands, isFree, read, write); err != nil || p != 2222 { - t.Fatalf("stale-persisted re-claim = %d / %v, want 2222", p, err) - } }