test: raise coverage to 78% (target: 95) (#11)

Adds 3 test files covering:
- internal/tokens: Load (absent + present), Add+Find+Remove+Save roundtrip,
  FindByTypeNameEnv (case-insensitive type+name, empty-env-as-development
  fallback, empty-type guard), normalize() helper, auto-CreatedAt stamping,
  Save bad-path error, storePath() HOME resolution
- internal/secretstore: UseDefault non-clobbering of existing backend,
  UseDefault with disabled keychain returns nil, keychainBackend
  Available() env-disable path, Name() label, MemoryBackend Set("")
  deletes
- cmd: shortToken truncate logic, extractTagged tag-scan (request_id,
  upgrade, end-of-string, newline-terminator, absent-tag), humanMessage
  preference order (message > agent_action > code > empty)

Coverage: 69.0% -> 77.7%. Remaining gap is cmd/login.go pollForTierUpgrade
+ runUpgrade (require live API), cmd/root.go Execute + main.go (CLI
entrypoint glue), and secretstore/secretstore.go Get/Set/Delete against
the real OS keychain (intentionally excluded by INSTANT_DISABLE_KEYCHAIN
in tests to avoid touching the dev keychain).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: mastermanas805

cmd/coverage_helpers_test.go

@@ -0,0 +1,63 @@
+package cmd
+
+import (
+	"testing"
+)
+
+// TestShortToken covers the 0%-line in up.go.
+func TestShortToken(t *testing.T) {
+	cases := map[string]string{
+		"":              "",
+		"abc":           "abc",
+		"abcdefgh":      "abcdefgh",
+		"abcdefghi":     "abcdefgh",
+		"verylongtoken": "verylong",
+	}
+	for in, want := range cases {
+		got := shortToken(in)
+		if got != want {
+			t.Errorf("shortToken(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+// TestExtractTagged covers the partial-coverage helper in json_error.go.
+func TestExtractTagged(t *testing.T) {
+	s := "blah request_id=abc-123 other_tag=xx upgrade=https://foo trailing"
+	if got := extractTagged(s, "request_id="); got != "abc-123" {
+		t.Errorf("extractTagged request_id = %q", got)
+	}
+	if got := extractTagged(s, "upgrade="); got != "https://foo" {
+		t.Errorf("extractTagged upgrade = %q", got)
+	}
+	// Tag at end of string runs to EOS.
+	if got := extractTagged("x trailing=end", "trailing="); got != "end" {
+		t.Errorf("trailing tag = %q", got)
+	}
+	// Missing tag returns "".
+	if got := extractTagged(s, "absent="); got != "" {
+		t.Errorf("missing tag should yield empty, got %q", got)
+	}
+	// Stop on newline.
+	if got := extractTagged("v=stop\nmore", "v="); got != "stop" {
+		t.Errorf("newline should terminate, got %q", got)
+	}
+}
+
+// TestHumanMessage covers apierror.go humanMessage preference order.
+func TestHumanMessage(t *testing.T) {
+	cases := []struct {
+		env  apiErrorEnvelope
+		want string
+	}{
+		{apiErrorEnvelope{Message: "msg", AgentAction: "act", Error: "code"}, "msg"},
+		{apiErrorEnvelope{AgentAction: "act", Error: "code"}, "act"},
+		{apiErrorEnvelope{Error: "code"}, "code"},
+		{apiErrorEnvelope{}, ""},
+	}
+	for _, c := range cases {
+		if got := c.env.humanMessage(); got != c.want {
+			t.Errorf("humanMessage(%+v) = %q, want %q", c.env, got, c.want)
+		}
+	}
+}

internal/secretstore/keychain_test.go

@@ -0,0 +1,77 @@
+package secretstore
+
+import (
+	"testing"
+)
+
+// TestUseDefault_LeavesExistingBackend confirms UseDefault is non-clobbering.
+func TestUseDefault_LeavesExistingBackend(t *testing.T) {
+	m := UseMemoryBackend()
+	defer Use(nil)
+
+	if got := UseDefault(); got != m {
+		t.Errorf("UseDefault should preserve in-memory backend, got %T", got)
+	}
+}
+
+// TestUseDefault_NoBackend_DisabledKeychain — when the env var disables the
+// keychain and no backend is active, UseDefault returns nil.
+func TestUseDefault_DisabledKeychain(t *testing.T) {
+	Use(nil)
+	t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1")
+	got := UseDefault()
+	if got != nil {
+		t.Errorf("UseDefault with disabled keychain should return nil, got %T", got)
+	}
+}
+
+// TestKeychainBackend_Available_DisabledByEnv verifies the env override.
+func TestKeychainBackend_Available_DisabledByEnv(t *testing.T) {
+	t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1")
+	k := &keychainBackend{}
+	if k.Available() {
+		t.Error("keychain should report unavailable when INSTANT_DISABLE_KEYCHAIN=1")
+	}
+	if k.Name() != "keychain" {
+		t.Errorf("Name() = %q", k.Name())
+	}
+}
+
+// TestKeychainBackend_SetEmptyDeletes covers the "Set("") -> Delete" branch
+// even when the keychain itself is unavailable (calling Delete is idempotent
+// and safe to invoke against a missing/unavailable backend).
+func TestKeychainBackend_SetEmptyDeletes(t *testing.T) {
+	t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1")
+	k := &keychainBackend{}
+	// Set("") routes to Delete(); Delete must be idempotent, so this should
+	// either return nil or a non-fatal error from go-keyring on hosts without
+	// a keychain. Just ensure it doesn't panic.
+	_ = k.Set("")
+}
+
+// TestMemoryBackend_Direct exercises *MemoryBackend methods through the type.
+func TestMemoryBackend_Direct(t *testing.T) {
+	m := &MemoryBackend{}
+	if _, err := m.Get(); err != ErrNotFound {
+		t.Errorf("empty Get should return ErrNotFound, got %v", err)
+	}
+	if err := m.Set("x"); err != nil {
+		t.Fatalf("Set: %v", err)
+	}
+	if v, _ := m.Get(); v != "x" {
+		t.Errorf("Get = %q", v)
+	}
+	if err := m.Set(""); err != nil {
+		t.Fatalf("Set empty: %v", err)
+	}
+	if _, err := m.Get(); err != ErrNotFound {
+		t.Error("Set(\"\") should clear")
+	}
+	if !m.Available() {
+		t.Error("MemoryBackend always available")
+	}
+	if m.Name() != "memory" {
+		t.Errorf("Name = %q", m.Name())
+	}
+	_ = m.Delete()
+}

internal/tokens/store_test.go

@@ -0,0 +1,161 @@
+package tokens
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// setupTempHome redirects HOME to a temp dir for the test, ensuring
+// storePath() lands in an isolated location.
+func setupTempHome(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	return dir
+}
+
+func TestLoad_AbsentFile(t *testing.T) {
+	setupTempHome(t)
+	s, err := Load()
+	if err != nil {
+		t.Fatalf("Load: %v", err)
+	}
+	if len(s.Entries) != 0 {
+		t.Errorf("expected empty entries, got %d", len(s.Entries))
+	}
+}
+
+func TestAdd_Find_Remove_Save(t *testing.T) {
+	dir := setupTempHome(t)
+	s, err := Load()
+	if err != nil {
+		t.Fatalf("Load: %v", err)
+	}
+
+	if err := s.Add(Entry{Token: "tok1", Name: "db1", Type: "postgres", URL: "postgres://x"}); err != nil {
+		t.Fatalf("Add: %v", err)
+	}
+	if err := s.Add(Entry{Token: "tok2", Name: "c1", Type: "redis", URL: "redis://x", CreatedAt: time.Now().Add(-time.Hour)}); err != nil {
+		t.Fatalf("Add: %v", err)
+	}
+
+	// Find roundtrips
+	if e := s.Find("tok1"); e == nil || e.Name != "db1" {
+		t.Errorf("Find(tok1) = %+v", e)
+	}
+	if e := s.Find("missing"); e != nil {
+		t.Errorf("Find(missing) = %+v, want nil", e)
+	}
+
+	// File written
+	if _, err := os.Stat(filepath.Join(dir, ".instant-tokens")); err != nil {
+		t.Errorf("expected file written, got %v", err)
+	}
+
+	// Reload preserves data
+	s2, err := Load()
+	if err != nil {
+		t.Fatalf("Load2: %v", err)
+	}
+	if len(s2.Entries) != 2 {
+		t.Errorf("expected 2 entries, got %d", len(s2.Entries))
+	}
+
+	// Remove existing & missing
+	if !s2.Remove("tok1") {
+		t.Error("Remove(existing) should return true")
+	}
+	if s2.Remove("nope") {
+		t.Error("Remove(missing) should return false")
+	}
+	if s2.Find("tok1") != nil {
+		t.Error("tok1 should be gone")
+	}
+}
+
+func TestFindByTypeNameEnv(t *testing.T) {
+	setupTempHome(t)
+	s, _ := Load()
+	_ = s.Add(Entry{Token: "tok1", Name: "DB One", Type: "Postgres", Env: "production"})
+	_ = s.Add(Entry{Token: "tok2", Name: "cache", Type: "redis", Env: ""}) // legacy empty env
+	_ = s.Add(Entry{Token: "tok3", Name: "x", Type: ""})                   // legacy empty type — ignored
+
+	// Case-insensitive type+name match.
+	if e := s.FindByTypeNameEnv("postgres", "db one", "production"); e == nil || e.Token != "tok1" {
+		t.Errorf("expected tok1, got %+v", e)
+	}
+	// Empty cached env matches default "development".
+	if e := s.FindByTypeNameEnv("redis", "cache", "development"); e == nil || e.Token != "tok2" {
+		t.Errorf("expected tok2, got %+v", e)
+	}
+	// Empty cached env also matches empty.
+	if e := s.FindByTypeNameEnv("redis", "cache", ""); e == nil {
+		t.Errorf("empty env should match")
+	}
+	// Different env → no match.
+	if e := s.FindByTypeNameEnv("postgres", "db one", "staging"); e != nil {
+		t.Errorf("staging should not match production entry, got %+v", e)
+	}
+	// Empty type entry is never returned.
+	if e := s.FindByTypeNameEnv("", "x", ""); e != nil {
+		t.Errorf("empty-type entry returned: %+v", e)
+	}
+	// Unknown combo.
+	if e := s.FindByTypeNameEnv("kafka", "nope", "dev"); e != nil {
+		t.Errorf("expected nil for unknown, got %+v", e)
+	}
+}
+
+func TestNormalize_Internal(t *testing.T) {
+	cases := map[string]string{
+		"":         "",
+		"  Foo  ":  "foo",
+		"a\tB\nC":  "abc",
+		"POSTGRES": "postgres",
+	}
+	for in, want := range cases {
+		got := normalize(in)
+		if got != want {
+			t.Errorf("normalize(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+func TestAdd_AutoTimestamps(t *testing.T) {
+	setupTempHome(t)
+	s, _ := Load()
+	before := time.Now().UTC()
+	if err := s.Add(Entry{Token: "auto1"}); err != nil {
+		t.Fatalf("Add: %v", err)
+	}
+	e := s.Find("auto1")
+	if e == nil {
+		t.Fatal("Find returned nil")
+	}
+	if e.CreatedAt.IsZero() {
+		t.Error("CreatedAt should be auto-populated")
+	}
+	if e.CreatedAt.Before(before.Add(-time.Second)) {
+		t.Errorf("CreatedAt %v predates test start %v", e.CreatedAt, before)
+	}
+}
+
+func TestStorePath_Internal(t *testing.T) {
+	dir := setupTempHome(t)
+	p, err := storePath()
+	if err != nil {
+		t.Fatalf("storePath: %v", err)
+	}
+	if p != filepath.Join(dir, ".instant-tokens") {
+		t.Errorf("storePath = %q, want %q", p, filepath.Join(dir, ".instant-tokens"))
+	}
+}
+
+func TestSave_BadPath(t *testing.T) {
+	s := &Store{path: "/nonexistent-directory/no/such/file"}
+	if err := s.Save(); err == nil {
+		t.Error("expected error writing to bogus path")
+	}
+}