From cc83802244ea1a73781fcae245488494936b0596 Mon Sep 17 00:00:00 2001
From: mirivlad <mirvtop@yandex.ru>
Date: Tue, 18 Aug 2026 18:46:08 +0800
Subject: [PATCH] fix: register the forward add local-port flag
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`sshkeeper forward add` could never succeed. RunE read --local-port and init()
marked it required, but the flag was never registered on forwardAddCmd. Cobra
ignores MarkFlagRequired for an unknown flag, and GetInt returns 0 for one, so
every invocation failed validation with "invalid local port 0: must be
1-65535". There was no argument combination that worked.

Register the flag so both the read and the required annotation bind to a real
option. README and the guide already documented --local-port, so the intent was
there from the start; only the registration was missing.

The existing tests missed this because they build a throwaway cobra.Command,
register the flags on it themselves, and pass it to forwardAddCmd.RunE — the
real command's flag set was never exercised. Add a test that parses argv into
forwardAddCmd's own flags, plus one pinning the required annotation. Both fail
against the unfixed command with "unknown flag: --local-port".

Present since c2edaa4, so v0.2.0 and v0.3.0 both ship it broken.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 cmd/forward.go      |  1 +
 cmd/forward_test.go | 81 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/cmd/forward.go b/cmd/forward.go
index af3729f..1e29953 100644
--- a/cmd/forward.go
+++ b/cmd/forward.go
@@ -199,6 +199,7 @@ func init() {
 	forwardAddCmd.Flags().String("name", "", "Forward name")
 	forwardAddCmd.Flags().String("description", "", "Forward description")
 	forwardAddCmd.Flags().String("local-addr", "127.0.0.1", "Listen address")
+	forwardAddCmd.Flags().Int("local-port", 0, "Listen port")
 	forwardAddCmd.MarkFlagRequired("local-port")
 	forwardAddCmd.Flags().String("remote-addr", "", "Target address")
 	forwardAddCmd.Flags().Int("remote-port", 0, "Target port")
diff --git a/cmd/forward_test.go b/cmd/forward_test.go
index 09f57ba..cda8ff6 100644
--- a/cmd/forward_test.go
+++ b/cmd/forward_test.go
@@ -7,8 +7,22 @@ import (
 	"github.com/mirivlad/sshkeeper/internal/db"
 	"github.com/mirivlad/sshkeeper/internal/model"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
+// restoreFlags returns every flag touched during the test back to its default.
+// The cobra commands are package-level singletons, so parsing argv into one
+// leaks state into whatever test runs next.
+func restoreFlags(t *testing.T, cmd *cobra.Command) {
+	t.Helper()
+	t.Cleanup(func() {
+		cmd.Flags().Visit(func(f *pflag.Flag) {
+			_ = f.Value.Set(f.DefValue)
+			f.Changed = false
+		})
+	})
+}
+
 func TestForwardEditUpdatesEnabledFlag(t *testing.T) {
 	testDB, err := db.Open(t.TempDir())
 	if err != nil {
@@ -110,3 +124,70 @@ func TestForwardAddStoresNameAndDescription(t *testing.T) {
 		t.Fatalf("unexpected forward metadata: %#v", forwards[0])
 	}
 }
+
+// TestForwardAddParsesItsOwnFlags drives the real forwardAddCmd flag set the
+// way the CLI does, instead of handing RunE a command built by the test.
+//
+// Regression: RunE read --local-port and init() marked it required, but the
+// flag was never registered on forwardAddCmd. Cobra silently ignores
+// MarkFlagRequired for an unknown flag and GetInt returns 0 for one, so every
+// real invocation died on "invalid local port 0" while the sibling tests --
+// which registered the flag on a throwaway command themselves -- kept passing.
+func TestForwardAddParsesItsOwnFlags(t *testing.T) {
+	testDB, err := db.Open(t.TempDir())
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	defer testDB.Close()
+
+	previousDB := appDB
+	appDB = testDB
+	t.Cleanup(func() { appDB = previousDB })
+
+	server := &model.Server{Alias: "web", Host: "web.example.org", Port: 22, User: "root", AuthMethod: model.AuthKey}
+	if err := appDB.CreateServer(server); err != nil {
+		t.Fatalf("create server: %v", err)
+	}
+
+	restoreFlags(t, forwardAddCmd)
+	if err := forwardAddCmd.Flags().Parse([]string{
+		"--name", "Local PostgreSQL",
+		"--type", "local",
+		"--local-port", "15432",
+		"--remote-addr", "db01.internal.example.com",
+		"--remote-port", "5432",
+	}); err != nil {
+		t.Fatalf("parse forward add flags: %v", err)
+	}
+
+	if err := forwardAddCmd.RunE(forwardAddCmd, []string{"web"}); err != nil {
+		t.Fatalf("add forward: %v", err)
+	}
+
+	forwards, err := appDB.GetForwards(server.ID)
+	if err != nil {
+		t.Fatalf("get forwards: %v", err)
+	}
+	if len(forwards) != 1 {
+		t.Fatalf("expected one forward, got %d", len(forwards))
+	}
+	got := forwards[0]
+	if got.LocalPort != 15432 {
+		t.Fatalf("local port not carried through: got %d, want 15432", got.LocalPort)
+	}
+	if got.Type != model.ForwardLocal || got.RemoteAddr != "db01.internal.example.com" || got.RemotePort != 5432 {
+		t.Fatalf("unexpected forward: %#v", got)
+	}
+}
+
+// TestForwardAddRequiresLocalPort pins the flag's registration and its required
+// annotation, which is what MarkFlagRequired silently failed to attach.
+func TestForwardAddRequiresLocalPort(t *testing.T) {
+	flag := forwardAddCmd.Flags().Lookup("local-port")
+	if flag == nil {
+		t.Fatal("forward add does not register --local-port")
+	}
+	if _, ok := flag.Annotations[cobra.BashCompOneRequiredFlag]; !ok {
+		t.Fatal("--local-port is registered but not marked required")
+	}
+}