refactor(cli): tighten session flow and review cleanup (#20)

* feat(init): add stignore presets and autodetect

* feat(sync): make syncthing rescan interval configurable

* fix(sync): isolate local syncthing logs and cleanup

* test(sync): expand syncthing coverage

* test: expand coverage for kube and syncthing helpers

* feat(sync): make syncthing relays configurable

* refactor(cli): extract ssh reconnect loop

* refactor(cli): split up flow and centralize command context

* refactor(cli): extend command context rollout

* fix: address remaining review findings

* refactor: tighten remaining review cleanup

* refactor(cli): finish remaining cleanup

* chore: stop tracking local review notes

* fix: keep review notes untracked without ignore rule

* test: expand coverage across helper modules

* test: raise kube and ssh coverage

* style(test): format kube coverage tests

* style: format remaining go files

Author: acmore

README.md

@@ -31,6 +31,14 @@ See:
 - `docs/troubleshooting.md`
 - `docs/release.md`
 
+## Development
+
+Coverage on some Go 1.25 toolchains can fail under `go test ./... -cover` because the bundled `covdata` tool is missing for packages without tests. Use the repo helper instead:
+
+```bash
+./scripts/coverage.sh
+```
+
 ## GitHub Pages Docs
 
 Docs are published from `docs/` via GitHub Actions + MkDocs.

cmd/okdev-sshd/main_test.go

@@ -1,6 +1,10 @@
 package main
 
 import (
+	"errors"
+	"os"
+	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/gliderlabs/ssh"
@@ -37,3 +41,118 @@ func TestNewServerAddsPublicKeyHandlerWhenKeysProvided(t *testing.T) {
 		t.Fatal("expected public key handler to be configured")
 	}
 }
+
+func TestDetectShellReturnsExistingShell(t *testing.T) {
+	got := detectShell()
+	if got != "/bin/bash" && got != "/bin/sh" {
+		t.Fatalf("unexpected shell %q", got)
+	}
+	if _, err := os.Stat(got); err != nil {
+		t.Fatalf("expected detected shell to exist: %v", err)
+	}
+}
+
+func TestLoadAuthorizedKeysMissingFileReturnsNil(t *testing.T) {
+	keys, err := loadAuthorizedKeys("/definitely/missing/authorized_keys")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if keys != nil {
+		t.Fatalf("expected nil keys for missing file, got %d", len(keys))
+	}
+}
+
+func TestLoadAuthorizedKeysParsesMultipleKeys(t *testing.T) {
+	path := t.TempDir() + "/authorized_keys"
+	content := strings.Join([]string{
+		"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIE9mN6e2Q2x8tQz4pT2r8j04YfGLwRoTSesFiNUFDXL9 test1",
+		"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJ2uL4N6OQ9bQG6tW1c2GqU2o6L1Qm0f0g5d2sWlY4Hn test2",
+		"",
+	}, "\n")
+	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	keys, err := loadAuthorizedKeys(path)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(keys) != 2 {
+		t.Fatalf("unexpected key count: got %d want 2", len(keys))
+	}
+}
+
+func TestLoadAuthorizedKeysRejectsInvalidData(t *testing.T) {
+	path := t.TempDir() + "/authorized_keys"
+	if err := os.WriteFile(path, []byte("not a key\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := loadAuthorizedKeys(path); err == nil {
+		t.Fatal("expected parse error")
+	}
+}
+
+func TestBuildInteractiveLoginScript(t *testing.T) {
+	script := buildInteractiveLoginScript(
+		map[string]string{},
+		"/bin/sh",
+		"/workspace/demo",
+		"1",
+	)
+
+	for _, want := range []string{
+		"cd '/workspace/demo'",
+		"/workspace/demo/.okdev/post-attach.sh",
+		"tmux",
+		"exec '/bin/sh' -l",
+	} {
+		if !strings.Contains(script, want) {
+			t.Fatalf("expected script to contain %q: %s", want, script)
+		}
+	}
+}
+
+func TestBuildInteractiveLoginScriptSkipsTmuxWhenDisabled(t *testing.T) {
+	script := buildInteractiveLoginScript(
+		map[string]string{"OKDEV_NO_TMUX": "1"},
+		"/bin/bash",
+		"",
+		"1",
+	)
+
+	if strings.Contains(script, "tmux") {
+		t.Fatalf("expected tmux bootstrap to be skipped: %s", script)
+	}
+	if !strings.Contains(script, "exec '/bin/bash' -l") {
+		t.Fatalf("expected login shell exec: %s", script)
+	}
+}
+
+func TestShellQuoteEscapesSingleQuotes(t *testing.T) {
+	if got := shellQuote("a'b"); got != `'a'"'"'b'` {
+		t.Fatalf("unexpected quoted string: %s", got)
+	}
+	if got := shellQuote(""); got != "''" {
+		t.Fatalf("unexpected empty quoted string: %s", got)
+	}
+}
+
+func TestExitStatus(t *testing.T) {
+	if got := exitStatus(nil); got != 0 {
+		t.Fatalf("unexpected nil exit status: %d", got)
+	}
+
+	if got := exitStatus(errors.New("plain")); got != 1 {
+		t.Fatalf("unexpected generic exit status: %d", got)
+	}
+
+	cmd := exec.Command("/bin/sh", "-c", "exit 7")
+	err := cmd.Run()
+	if err == nil {
+		t.Fatal("expected exit error")
+	}
+	if got := exitStatus(err); got != 7 {
+		t.Fatalf("unexpected exit status: got %d want 7", got)
+	}
+}

docs/config-manifest.md

@@ -165,6 +165,8 @@ spec:
 | `syncthing.version` | `string` | `v1.29.7` | Local Syncthing binary version |
 | `syncthing.autoInstall` | `bool` | `true` | Auto-install local Syncthing |
 | `syncthing.image` | `string` | `ghcr.io/acmore/okdev:<version>` | Sidecar image (fallback: `edge`) |
+| `syncthing.rescanIntervalSeconds` | `int` | `300` | Fallback full rescan interval; `0` disables periodic rescans |
+| `syncthing.relaysEnabled` | `bool` | `false` | Allow Syncthing relay fallback when direct connectivity is unavailable |
 
 **Validation:** `engine` must be `syncthing`; each `paths[]` entry must be `local:remote`.
 
@@ -177,6 +179,8 @@ spec:
     syncthing:
       version: v1.29.7
       autoInstall: true
+      rescanIntervalSeconds: 300
+      relaysEnabled: false
     paths:
       - .:/workspace
     remoteExclude:

docs/quickstart.md

@@ -126,6 +126,7 @@ okdev sync --mode down
 ```
 
 Local Syncthing is auto-installed when `sync.engine=syncthing`.
+If your local filesystem watcher occasionally misses changes, set `spec.sync.syncthing.rescanIntervalSeconds` to a lower value such as `300` for a faster fallback rescan.
 
 ## Port Forwarding
 

internal/cli/common.go

@@ -25,11 +25,19 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
-const sessionHeartbeatInterval = 5 * time.Minute
-const sshPort = 2222
-
 var invalidOwnerChars = regexp.MustCompile(`[^a-z0-9._-]`)
 
+type sessionResolver func(*Options, *config.DevEnvironment, string) (string, error)
+
+type commandContext struct {
+	opts        *Options
+	cfg         *config.DevEnvironment
+	cfgPath     string
+	namespace   string
+	sessionName string
+	kube        *kube.Client
+}
+
 func loadConfigAndNamespace(opts *Options) (*config.DevEnvironment, string, error) {
 	path, err := config.ResolvePath(opts.ConfigPath)
 	if err != nil {
@@ -56,6 +64,33 @@ func loadConfigAndNamespace(opts *Options) (*config.DevEnvironment, string, erro
 	return cfg, ns, nil
 }
 
+func resolveCommandContext(opts *Options, resolver sessionResolver) (*commandContext, error) {
+	cfg, namespace, err := loadConfigAndNamespace(opts)
+	if err != nil {
+		return nil, err
+	}
+	cfgPath, err := config.ResolvePath(opts.ConfigPath)
+	if err != nil {
+		return nil, err
+	}
+	cc := &commandContext{
+		opts:      opts,
+		cfg:       cfg,
+		cfgPath:   cfgPath,
+		namespace: namespace,
+		kube:      newKubeClient(opts),
+	}
+	if resolver == nil {
+		return cc, nil
+	}
+	sessionName, err := resolver(opts, cfg, namespace)
+	if err != nil {
+		return nil, err
+	}
+	cc.sessionName = sessionName
+	return cc, nil
+}
+
 func withQuietConfigAnnounce(fn func() error) error {
 	prevQuiet, hadQuiet := os.LookupEnv("OKDEV_QUIET_CONFIG_ANNOUNCE")
 	_ = os.Setenv("OKDEV_QUIET_CONFIG_ANNOUNCE", "1")
@@ -139,7 +174,7 @@ func newTransientStatusWithMode(w io.Writer, message string, interactive bool) *
 	s.enabled = true
 	s.stopCh = make(chan struct{})
 	s.doneCh = make(chan struct{})
-	go s.run(120 * time.Millisecond)
+	go s.run(statusSpinnerInterval)
 	return s
 }
 
@@ -225,10 +260,6 @@ func resolveSessionName(opts *Options, cfg *config.DevEnvironment, namespace str
 	return resolveSessionNameWithState(opts, cfg, namespace, true)
 }
 
-func resolveSessionNameForUpDown(opts *Options, cfg *config.DevEnvironment, namespace string) (string, error) {
-	return resolveSessionNameWithState(opts, cfg, namespace, true)
-}
-
 func resolveSessionNameWithState(opts *Options, cfg *config.DevEnvironment, namespace string, inferExisting bool) (string, error) {
 	return resolveSessionNameWithReader(opts, cfg, namespace, inferExisting, newKubeClient(opts))
 }
@@ -251,7 +282,9 @@ func resolveSessionNameWithReader(opts *Options, cfg *config.DevEnvironment, nam
 			if exists {
 				return resolvedActive, nil
 			}
-			_ = session.ClearActiveSession()
+			if clearErr := session.ClearActiveSession(); clearErr != nil {
+				slog.Debug("failed to clear stale active session", "session", resolvedActive, "error", clearErr)
+			}
 		} else {
 			slog.Debug("failed to verify active session pod", "session", resolvedActive, "namespace", namespace, "error", existsErr)
 			return resolvedActive, nil
@@ -270,7 +303,7 @@ func resolveSessionNameWithReader(opts *Options, cfg *config.DevEnvironment, nam
 }
 
 func sessionPodExists(k sessionAccessReader, namespace, sessionName string) (bool, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), sessionExistsTimeout)
 	defer cancel()
 	pods, err := k.ListPods(ctx, namespace, false, "okdev.io/managed=true,okdev.io/session="+sessionName)
 	if err == nil {
@@ -303,7 +336,7 @@ func inferExistingSessionForRepo(opts *Options, cfg *config.DevEnvironment, name
 	if strings.TrimSpace(cfg.Metadata.Name) != "" {
 		label = append(label, "okdev.io/name="+cfg.Metadata.Name)
 	}
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), sessionExistsTimeout)
 	defer cancel()
 	pods, err := newKubeClient(opts).ListPods(ctx, namespace, false, strings.Join(label, ","))
 	if err != nil {
@@ -376,7 +409,7 @@ func newKubeClient(opts *Options) *kube.Client {
 }
 
 func defaultContext() (context.Context, context.CancelFunc) {
-	return context.WithTimeout(context.Background(), 5*time.Minute)
+	return context.WithTimeout(context.Background(), defaultContextTimeout)
 }
 
 func interactiveContext() (context.Context, context.CancelFunc) {
@@ -394,13 +427,11 @@ func runConnectWithClient(k *kube.Client, namespace string, target workload.Targ
 	return connect.Run(ctx, k, namespace, target.PodName, command, tty, os.Stdin, os.Stdout, os.Stderr)
 }
 
-func startSessionMaintenance(opts *Options, cfg *config.DevEnvironment, namespace, sessionName string, out io.Writer, renewLock bool, emitHeartbeat bool) func() {
-	return startSessionMaintenanceWithClient(newKubeClient(opts), cfg, namespace, sessionName, out, renewLock, emitHeartbeat)
+func startSessionMaintenance(opts *Options, namespace, sessionName string, out io.Writer, emitHeartbeat bool) func() {
+	return startSessionMaintenanceWithClient(newKubeClient(opts), namespace, sessionName, out, emitHeartbeat)
 }
 
-func startSessionMaintenanceWithClient(k *kube.Client, cfg *config.DevEnvironment, namespace, sessionName string, out io.Writer, renewLock bool, emitHeartbeat bool) func() {
-	_ = cfg
-	_ = renewLock
+func startSessionMaintenanceWithClient(k *kube.Client, namespace, sessionName string, out io.Writer, emitHeartbeat bool) func() {
 	if !emitHeartbeat {
 		return func() {}
 	}
@@ -410,13 +441,15 @@ func startSessionMaintenanceWithClient(k *kube.Client, cfg *config.DevEnvironmen
 		pod = target.PodName
 	}
 	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
 
 	go func() {
+		defer close(done)
 		heartbeatTicker := time.NewTicker(sessionHeartbeatInterval)
 		defer heartbeatTicker.Stop()
 
 		doHeartbeat := func() {
-			beatCtx, beatCancel := context.WithTimeout(context.Background(), 10*time.Second)
+			beatCtx, beatCancel := context.WithTimeout(context.Background(), heartbeatWriteTimeout)
 			err := k.TouchPodActivity(beatCtx, namespace, pod)
 			beatCancel()
 			if err != nil {
@@ -436,7 +469,10 @@ func startSessionMaintenanceWithClient(k *kube.Client, cfg *config.DevEnvironmen
 		}
 	}()
 
-	return cancel
+	return func() {
+		cancel()
+		<-done
+	}
 }
 
 func currentOwner(opts *Options) string {
@@ -487,7 +523,7 @@ type sessionAccessReader interface {
 }
 
 func ensureSessionAccess(opts *Options, k sessionAccessReader, namespace, sessionName string, allowShareable bool, requireExisting bool) error {
-	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), sessionAccessTimeout)
 	defer cancel()
 	pods, err := k.ListPods(ctx, namespace, false, "okdev.io/managed=true,okdev.io/session="+sessionName)
 	if err != nil {

internal/cli/connect.go

@@ -17,23 +17,18 @@ func newConnectCmd(opts *Options) *cobra.Command {
 		Args:  cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			applySessionArg(opts, args)
-			cfg, ns, err := loadConfigAndNamespace(opts)
+			cc, err := resolveCommandContext(opts, resolveSessionName)
 			if err != nil {
 				return err
 			}
-			sn, err := resolveSessionName(opts, cfg, ns)
-			if err != nil {
-				return err
-			}
-			k := newKubeClient(opts)
-			if err := ensureExistingSessionOwnership(opts, k, ns, sn, true); err != nil {
+			if err := ensureExistingSessionOwnership(opts, cc.kube, cc.namespace, cc.sessionName, true); err != nil {
 				return err
 			}
-			target, err := resolveTargetRef(cmd.Context(), opts, cfg, ns, sn, k)
+			target, err := resolveTargetRef(cmd.Context(), opts, cc.cfg, cc.namespace, cc.sessionName, cc.kube)
 			if err != nil {
 				return err
 			}
-			stopMaintenance := startSessionMaintenance(opts, cfg, ns, sn, cmd.OutOrStdout(), true, true)
+			stopMaintenance := startSessionMaintenance(opts, cc.namespace, cc.sessionName, cmd.OutOrStdout(), true)
 			defer stopMaintenance()
 
 			execCmd := []string{"sh", "-lc", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"}
@@ -49,7 +44,7 @@ func newConnectCmd(opts *Options) *cobra.Command {
 			if len(execCmd) == 1 && strings.TrimSpace(execCmd[0]) == "" {
 				execCmd = []string{"sh", "-lc", "command -v bash >/dev/null 2>&1 && exec bash || exec sh"}
 			}
-			return runConnectWithClient(k, ns, target, execCmd, !noTTY)
+			return runConnectWithClient(cc.kube, cc.namespace, target, execCmd, !noTTY)
 		},
 	}
 	cmd.Flags().StringVar(&shell, "shell", "", "Shell to start (default auto-detects bash/sh)")