ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers

.github/dependabot.yml

@@ -0,0 +1,41 @@
+# Dependabot config for tracebloc/cli — extends the org's backend-only
+# pilot (backend runs security-only updates; see tracebloc/backend
+# .github/dependabot.yml) to a second repo. Unlike backend, this repo
+# takes real VERSION updates: the CLI ships as a customer-installed
+# binary, so staying current on k8s.io/* and golang.org/x/* is a
+# security posture, not just hygiene (see #276 — six reachable vulns
+# rode a stale x/net + toolchain).
+#
+# gomod is weekly and grouped so the k8s.io/* constellation (which only
+# upgrades cleanly in lockstep) and the golang.org/x/* siblings arrive
+# as one PR each instead of a dozen singletons. github-actions is
+# monthly — workflow pins move rarely and reviews are trivial.
+#
+# Org-wide rollout beyond backend + cli is an open decision — flagged
+# to @saadqbal on the PR that added this file.
+
+version: 2
+updates:
+ - package-ecosystem: "gomod"
+ directory: "/"
+ schedule:
+ interval: "weekly"
+ labels:
+ - "work-type:tech-debt"
+ - "dependencies"
+ groups:
+ k8s:
+ patterns:
+ - "k8s.io/*"
+ - "sigs.k8s.io/*"
+ golang-x:
+ patterns:
+ - "golang.org/x/*"
+
+ - package-ecosystem: "github-actions"
+ directory: "/"
+ schedule:
+ interval: "monthly"
+ labels:
+ - "work-type:tech-debt"
+ - "dependencies"

.github/workflows/build.yml

@@ -129,6 +129,20 @@ jobs:
  exit 1
  fi
 
+ # goimports -local: enforce the stdlib / third-party / our-own import
+ # grouping that .golangci.yml's local-prefixes already declares. gofmt
+ # doesn't check grouping, so drift accumulated silently until now.
+ - name: goimports -local
+ run: |
+ go install golang.org/x/tools/cmd/goimports@v0.48.0
+ drift="$(goimports -local github.com/tracebloc/cli -l .)"
+ if [ -n "$drift" ]; then
+ echo "::error::goimports (import grouping) drift in:"
+ echo "$drift" | sed 's/^/ /'
+ echo "::error::run \`make fmt\` to fix"
+ exit 1
+ fi
+
  - name: ineffassign
  run: |
  go install github.com/gordonklaus/ineffassign@v0.2.0
@@ -149,19 +163,16 @@ jobs:
  go install honnef.co/go/tools/cmd/staticcheck@2025.1.1
  staticcheck -checks all,-ST1005 ./...
 
- # deadcode: reachability scan from the CLI entrypoint (~5s). ADVISORY
- # (continue-on-error) for now — it reports unreachable funcs but does not
- # fail the job. The module still carries pre-existing dead-ish funcs that
- # are unsafe to delete blindly: Stringer methods (Status.String,
- # JobOutcome.String) reached only via fmt reflection that static analysis
- # can't see, plus test-only parity harnesses (ReadLabelValues,
- # inferColumnType — di#349). Flip to blocking once that backlog is
- # cleared. Tracked in #6 / #127.
- - name: deadcode (advisory)
- continue-on-error: true
- run: |
- go install golang.org/x/tools/cmd/deadcode@v0.48.0
- deadcode ./cmd/tracebloc
+ # deadcode: BLOCKING reachability scan from the CLI entrypoint (~5s).
+ # The four legit unreachables — Stringer methods (Status.String,
+ # JobOutcome.String) reached only via fmt reflection that static
+ # analysis can't see, plus the di#349 test-only parity harnesses
+ # (ReadLabelValues, inferColumnType) — are declared with reasons in
+ # scripts/deadcode-allowlist.txt. Anything else unreachable fails the
+ # job (#281 flipped this from advisory/continue-on-error). Tool version
+ # pinned inside the script; DEADCODE_VERSION overrides.
+ - name: deadcode
+ run: ./scripts/deadcode-check.sh
 
  govulncheck:
  timeout-minutes: 10

Makefile

@@ -20,6 +20,7 @@ MISSPELL_VERSION ?= v0.3.4
 DEADCODE_VERSION ?= v0.48.0
 GOVULNCHECK_VERSION ?= v1.1.4
 STATICCHECK_VERSION ?= 2025.1.1
+GOIMPORTS_VERSION ?= v0.48.0
 
 # ---- top-level targets -------------------------------------------
 
@@ -106,17 +107,15 @@ lint:
 	$(GO) run github.com/client9/misspell/cmd/misspell@$(MISSPELL_VERSION) -error .
 	$(GO) run honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) -checks all,-ST1005 ./...
 
-# deadcode: reachability scan from the CLI entrypoint (~5s). ADVISORY for now
-# (non-blocking) — it prints unreachable funcs but never fails the build. The
-# module still carries pre-existing dead-ish funcs that are unsafe to delete
-# blindly: Stringer methods (Status.String, JobOutcome.String) reached only via
-# fmt reflection that static analysis can't see, plus test-only parity harnesses
-# (ReadLabelValues, inferColumnType — di#349). Flip to blocking once that
-# backlog is cleared. Tracked in tracebloc/cli#6 / #127.
+# deadcode: BLOCKING reachability scan from the CLI entrypoint (~5s). The four
+# legit unreachables — Stringer methods (Status.String, JobOutcome.String)
+# reached only via fmt reflection that static analysis can't see, plus the
+# di#349 test-only parity harnesses (ReadLabelValues, inferColumnType) — are
+# declared in scripts/deadcode-allowlist.txt with reasons. Anything else
+# unreachable fails the build (#281 flipped this from advisory).
 .PHONY: deadcode
 deadcode:
-	@echo "==> deadcode (advisory): unreachable funcs from ./cmd/tracebloc"
-	@$(GO) run golang.org/x/tools/cmd/deadcode@$(DEADCODE_VERSION) ./cmd/tracebloc || true
+	@DEADCODE_VERSION=$(DEADCODE_VERSION) ./scripts/deadcode-check.sh
 
 # vulncheck: govulncheck reachability scan for known CVEs (stdlib + deps).
 # BLOCKING — this is a customer-installed binary; v0.8.0 shipped with 6
@@ -140,7 +139,10 @@ lint-full:
 .PHONY: fmt
 fmt:
 	gofmt -s -w .
+	$(GO) run golang.org/x/tools/cmd/goimports@$(GOIMPORTS_VERSION) -local github.com/tracebloc/cli -w .
 
+# fmt-check: gofmt -s (simplification) + goimports -local (import grouping:
+# stdlib / third-party / our own — matches .golangci.yml's local-prefixes).
 .PHONY: fmt-check
 fmt-check:
 	@diff="$$(gofmt -s -l . 2>/dev/null)"; \
@@ -150,6 +152,13 @@ fmt-check:
  echo "==> run \`make fmt\` to fix"; \
  exit 1; \
 	fi
+	@drift="$$($(GO) run golang.org/x/tools/cmd/goimports@$(GOIMPORTS_VERSION) -local github.com/tracebloc/cli -l .)"; \
+	if [ -n "$$drift" ]; then \
+ echo "==> goimports (import grouping) needed on:"; \
+ echo "$$drift" | sed 's/^/ /'; \
+ echo "==> run \`make fmt\` to fix"; \
+ exit 1; \
+	fi
 
 .PHONY: schema-check
 schema-check:

internal/cli/data_test.go

@@ -1,22 +1,23 @@
 package cli
 
 import (
-	"github.com/tracebloc/cli/internal/push"
-	"github.com/tracebloc/cli/internal/ui"
-	"image"
-	"image/png"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/rest"
-
 	"bytes"
 	"context"
 	"errors"
-	"github.com/tracebloc/cli/internal/cluster"
+	"image"
+	"image/png"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
+
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+
+	"github.com/tracebloc/cli/internal/cluster"
+	"github.com/tracebloc/cli/internal/push"
+	"github.com/tracebloc/cli/internal/ui"
 )
 
 // imgcLayout drops a minimum-viable image_classification directory

internal/cli/ingestion_run_test.go

@@ -10,12 +10,13 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+
 	"github.com/tracebloc/cli/internal/cluster"
 	"github.com/tracebloc/cli/internal/push"
 	"github.com/tracebloc/cli/internal/submit"
 	"github.com/tracebloc/cli/internal/ui"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/rest"
 )
 
 // The money path (#1009): submit → classify → exit-code → JSON → reclaim.

internal/cli/resources_set_test.go

@@ -7,10 +7,11 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/client-go/kubernetes/fake"
+
 	"github.com/tracebloc/cli/internal/cluster"
 	"github.com/tracebloc/cli/internal/helm"
 	"github.com/tracebloc/cli/internal/ui"
-	"k8s.io/client-go/kubernetes/fake"
 )
 
 // setTarget builds a resolved cluster target from a fake clientset, with a chart

internal/config/config.go

@@ -234,18 +234,3 @@ func (c *Config) Save() error {
 	}
 	return nil
 }
-
-// clearAll removes the config file (full sign-out + reset, all envs). A missing
-// file is not an error. Unexported: no production caller (logout uses Save);
-// retained only for the same-package test that pins the remove-and-tolerate-
-// missing behavior. (`clear` is a Go builtin, hence clearAll.)
-func clearAll() error {
-	path, err := Path()
-	if err != nil {
- return err
-	}
-	if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) {
- return fmt.Errorf("removing %s: %w", path, err)
-	}
-	return nil
-}

internal/config/config_coverage_test.go

@@ -9,7 +9,7 @@ import (
 
 // clearHomeAndConfigDir removes every source of a config dir so Dir()/Path()
 // fail — the lever that covers the error-propagation branches of Dir, Path,
-// Load, Save and clearAll (os.UserHomeDir errors when $HOME is empty).
+// Loadand Save (os.UserHomeDir errors when $HOME is empty).
 func clearHomeAndConfigDir(t *testing.T) {
 	t.Helper()
 	t.Setenv("TRACEBLOC_CONFIG_DIR", "")
@@ -148,10 +148,3 @@ func TestSave_ErrorBranches(t *testing.T) {
  }
 	})
 }
-
-func TestClearAll_HomeError(t *testing.T) {
-	clearHomeAndConfigDir(t)
-	if err := clearAll(); err == nil {
- t.Error("clearAll() must propagate Path()'s error")
-	}
-}

internal/config/config_test.go

@@ -55,20 +55,6 @@ func TestLoadMissingIsEmpty(t *testing.T) {
 	}
 }
 
-func TestClear(t *testing.T) {
-	t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir())
-	_ = (&Config{CurrentEnv: "prod", Profiles: map[string]*Profile{"prod": {Token: "x"}}}).Save()
-	if err := clearAll(); err != nil {
- t.Fatal(err)
-	}
-	if c, _ := Load(); c.SignedIn() {
- t.Error("after clearAll, should not be signed in")
-	}
-	if err := clearAll(); err != nil {
- t.Errorf("clearAll on a missing file should be nil, got %v", err)
-	}
-}
-
 // TestMigrateV1ToV2 pins the v1 (flat cli#83 schema) → v2 migration: the single
 // record is wrapped under profiles[env], no data loss, and the next Save rewrites
 // the file as v2 on disk.

internal/config/fault_test.go

@@ -91,43 +91,3 @@ func TestSave_Faults(t *testing.T) {
  }
 	})
 }
-
-// TestClearAll pins clearAll (config.go:242, was 67%): remove the config,
-// tolerate a missing file, and surface a genuine remove failure.
-func TestClearAll(t *testing.T) {
-	t.Run("removes an existing config", func(t *testing.T) {
- dir := t.TempDir()
- t.Setenv("TRACEBLOC_CONFIG_DIR", dir)
- path := filepath.Join(dir, "config.json")
- if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
- t.Fatal(err)
- }
- if err := clearAll(); err != nil {
- t.Fatalf("clearAll: %v", err)
- }
- if _, err := os.Stat(path); !os.IsNotExist(err) {
- t.Error("config must be gone after clearAll")
- }
-	})
-	t.Run("missing config is a no-op", func(t *testing.T) {
- t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir())
- if err := clearAll(); err != nil {
- t.Errorf("a missing config must clear cleanly, got %v", err)
- }
-	})
-	t.Run("un-removable path -> error", func(t *testing.T) {
- dir := t.TempDir()
- t.Setenv("TRACEBLOC_CONFIG_DIR", dir)
- // config.json as a NON-EMPTY directory → os.Remove refuses it.
- cfgDir := filepath.Join(dir, "config.json")
- if err := os.Mkdir(cfgDir, 0o700); err != nil {
- t.Fatal(err)
- }
- if err := os.WriteFile(filepath.Join(cfgDir, "child"), []byte("x"), 0o600); err != nil {
- t.Fatal(err)
- }
- if err := clearAll(); err == nil {
- t.Error("a non-empty directory at the config path must surface a remove error")
- }
-	})
-}

internal/push/category.go

@@ -299,16 +299,6 @@ func SupportedCategoryIDs() []string {
 	return ids
 }
 
-// allCategoryIDs returns every recognized category id, in registry order.
-// Unexported: only the same-package registry test consumes it.
-func allCategoryIDs() []string {
-	ids := make([]string, 0, len(categoryRegistry))
-	for _, c := range categoryRegistry {
- ids = append(ids, c.ID)
-	}
-	return ids
-}
-
 // SupportedCategoriesList is the comma-joined supported ids, for help text
 // and gate error messages.
 func SupportedCategoriesList() string { return strings.Join(SupportedCategoryIDs(), ", ") }

internal/push/category_registry_test.go

@@ -12,6 +12,17 @@ import (
 // that the family predicates + the supported set all derive from it, so a
 // future edit can't reintroduce the "5 of 9" drift (cli#74).
 
+// allCategoryIDs returns every recognized category id, in registry order.
+// Test-local: production code iterates categoryRegistry directly, so this
+// lives here to keep the shipped binary free of test-only helpers (#281).
+func allCategoryIDs() []string {
+	ids := make([]string, 0, len(categoryRegistry))
+	for _, c := range categoryRegistry {
+ ids = append(ids, c.ID)
+	}
+	return ids
+}
+
 func TestRegistryKnownCategories(t *testing.T) {
 	want := []string{
  "image_classification", "object_detection", "keypoint_detection",

internal/submit/client.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -184,12 +183,3 @@ func (e *SubmitError) Error() string {
 	return fmt.Sprintf("jobs-manager %s returned HTTP %d: %s",
  e.Endpoint, e.StatusCode, strings.TrimSpace(e.Body))
 }
-
-// isSubmitError reports whether err is a *SubmitError. Convenience
-// for the orchestrator's exit-code mapping; errors.As would also
-// work but this reads cleaner at the branch site. Unexported: only
-// same-package tests reference it today.
-func isSubmitError(err error) bool {
-	var se *SubmitError
-	return errors.As(err, &se)
-}

internal/submit/client_test.go

@@ -11,6 +11,15 @@ import (
 	"testing"
 )
 
+// isSubmitError reports whether err is a *SubmitError. Test-local
+// assertion helper (also used by submit_test.go): production code has
+// no caller, so it lives here to keep the shipped binary free of
+// test-only helpers (#281).
+func isSubmitError(err error) bool {
+	var se *SubmitError
+	return errors.As(err, &se)
+}
+
 // TestHTTPSubmitter_HappyPath: jobs-manager returns 201 with the
 // canonical body shape; client decodes correctly + surfaces all
 // three response fields.