test(backend): assert three mutation-surviving code paths (#2481)

Co-authored-by: Kandev Agent <agent@kandev.dev>

apps/backend/internal/backendapp/adapters_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sync"
 	"testing"
 
@@ -45,6 +46,201 @@ func TestBuildLifecycleLaunchRequestForwardsRemoteContributions(t *testing.T) {
 	}
 }
 
+// distinctFiller populates struct fields with non-zero values that are unique
+// per field, so a mapper that drops a field (or crosswires two of the same
+// type) produces an observable difference rather than two matching zero
+// values. Bool is the documented exception — see fill.
+type distinctFiller struct{ n int }
+
+// fillStruct populates every exported field of the struct pointed at by ptr.
+func (f *distinctFiller) fillStruct(t *testing.T, ptr any) {
+	t.Helper()
+	v := reflect.ValueOf(ptr).Elem()
+	typ := v.Type()
+	for i := 0; i < typ.NumField(); i++ {
+		field := typ.Field(i)
+		if !v.Field(i).CanSet() {
+			t.Fatalf("distinctFiller: %s.%s is unexported and cannot be covered", typ, field.Name)
+		}
+		f.fill(t, v.Field(i), field.Name)
+	}
+}
+
+func (f *distinctFiller) fill(t *testing.T, v reflect.Value, name string) {
+	t.Helper()
+	f.n++
+	switch v.Kind() {
+	case reflect.String:
+		v.SetString(fmt.Sprintf("%s-%d", name, f.n))
+	case reflect.Bool:
+		// Always true, never counter-derived: bool has only two values and one
+		// of them is the zero value, so distinctness across several bool fields
+		// and drop-detection are mutually exclusive. Drop-detection wins —
+		// a field left unmapped reads back false and fails. The cost is that
+		// two crosswired bool fields would still compare equal.
+		v.SetBool(true)
+	case reflect.Int, reflect.Int64:
+		v.SetInt(int64(f.n))
+	case reflect.Pointer:
+		// Deliberately shallow: a zero-value allocation is enough to catch a
+		// dropped pointer field (nil != non-nil). It does not catch a mapper
+		// that substitutes a freshly allocated zero value of the same type,
+		// since reflect.DeepEqual compares pointees rather than addresses.
+		v.Set(reflect.New(v.Type().Elem()))
+	case reflect.Slice:
+		slice := reflect.MakeSlice(v.Type(), 1, 1)
+		f.fill(t, slice.Index(0), name+"-elem")
+		v.Set(slice)
+	case reflect.Map:
+		m := reflect.MakeMap(v.Type())
+		key := reflect.New(v.Type().Key()).Elem()
+		f.fill(t, key, name+"-key")
+		value := reflect.New(v.Type().Elem()).Elem()
+		f.fill(t, value, name+"-value")
+		m.SetMapIndex(key, value)
+		v.Set(m)
+	default:
+		t.Fatalf("distinctFiller: unsupported kind %s for field %s — extend the helper", v.Kind(), name)
+	}
+}
+
+// assertSameNamedFieldsEqual fails unless src and dst expose the same number of
+// exported fields and every same-named pair holds a DeepEqual value. Mapper
+// tests use it so that dropping a field — or adding one to both sides of a
+// mirrored type pair and forgetting to wire it — fails instead of passing
+// silently the way a hand-written subset of field assertions would.
+func assertSameNamedFieldsEqual(t *testing.T, label string, src, dst any) {
+	t.Helper()
+	srcVal, dstVal := reflect.ValueOf(src), reflect.ValueOf(dst)
+	srcType, dstType := srcVal.Type(), dstVal.Type()
+	if srcType.NumField() != dstType.NumField() {
+		t.Fatalf("%s: %s has %d fields but %s has %d — the mirrored types drifted",
+			label, srcType, srcType.NumField(), dstType, dstType.NumField())
+	}
+	for i := 0; i < srcType.NumField(); i++ {
+		name := srcType.Field(i).Name
+		dstField := dstVal.FieldByName(name)
+		if !dstField.IsValid() {
+			t.Errorf("%s: %s has no field %s", label, dstType, name)
+			continue
+		}
+		want := srcVal.Field(i).Interface()
+		if got := dstField.Interface(); !reflect.DeepEqual(got, want) {
+			t.Errorf("%s: field %s = %#v, want %#v", label, name, got, want)
+		}
+	}
+}
+
+// TestLifecycleWorkspaceFoldersMapsEveryField covers the launch path's folder
+// projection. Every agent launch flows through it and a dropped field is
+// silent — no panic, no error, just missing agent configuration.
+func TestLifecycleWorkspaceFoldersMapsEveryField(t *testing.T) {
+	if got := lifecycleWorkspaceFolders(nil); got != nil {
+		t.Errorf("lifecycleWorkspaceFolders(nil) = %#v, want nil", got)
+	}
+	if got := lifecycleWorkspaceFolders([]orchestratorexecutor.WorkspaceFolderSpec{}); got != nil {
+		t.Errorf("lifecycleWorkspaceFolders(empty) = %#v, want nil", got)
+	}
+
+	filler := &distinctFiller{}
+	folders := make([]orchestratorexecutor.WorkspaceFolderSpec, 2)
+	for i := range folders {
+		filler.fillStruct(t, &folders[i])
+	}
+
+	got := lifecycleWorkspaceFolders(folders)
+	if len(got) != len(folders) {
+		t.Fatalf("lifecycleWorkspaceFolders returned %d folders, want %d", len(got), len(folders))
+	}
+	for i := range folders {
+		assertSameNamedFieldsEqual(t, fmt.Sprintf("workspace folder %d", i), folders[i], got[i])
+	}
+}
+
+// TestLifecycleRouteOverrideMapsEveryField pins the same all-fields contract on
+// the sibling route-override mapper.
+func TestLifecycleRouteOverrideMapsEveryField(t *testing.T) {
+	if got := lifecycleRouteOverride(nil); got != nil {
+		t.Errorf("lifecycleRouteOverride(nil) = %#v, want nil", got)
+	}
+
+	override := &orchestratorexecutor.RouteOverride{}
+	(&distinctFiller{}).fillStruct(t, override)
+
+	got := lifecycleRouteOverride(override)
+	if got == nil {
+		t.Fatal("lifecycleRouteOverride returned nil for a non-nil override")
+	}
+	assertSameNamedFieldsEqual(t, "route override", *override, *got)
+}
+
+// TestLifecycleRepoLaunchSpecsMapsEveryField pins the all-fields contract on
+// the multi-repo launch spec mapper — the widest of the three (19 fields).
+func TestLifecycleRepoLaunchSpecsMapsEveryField(t *testing.T) {
+	if got := lifecycleRepoLaunchSpecs(nil); got != nil {
+		t.Errorf("lifecycleRepoLaunchSpecs(nil) = %#v, want nil", got)
+	}
+	if got := lifecycleRepoLaunchSpecs([]orchestratorexecutor.RepoSpec{}); got != nil {
+		t.Errorf("lifecycleRepoLaunchSpecs(empty) = %#v, want nil", got)
+	}
+
+	filler := &distinctFiller{}
+	repos := make([]orchestratorexecutor.RepoSpec, 2)
+	for i := range repos {
+		filler.fillStruct(t, &repos[i])
+	}
+
+	got := lifecycleRepoLaunchSpecs(repos)
+	if len(got) != len(repos) {
+		t.Fatalf("lifecycleRepoLaunchSpecs returned %d specs, want %d", len(got), len(repos))
+	}
+	for i := range repos {
+		assertSameNamedFieldsEqual(t, fmt.Sprintf("repo spec %d", i), repos[i], got[i])
+	}
+}
+
+// TestBuildLifecycleLaunchRequestWiresMappedCollections asserts the launch
+// request actually carries the three mapper outputs. A mapper can be correct
+// while the wiring in buildLifecycleLaunchRequest drops it.
+func TestBuildLifecycleLaunchRequestWiresMappedCollections(t *testing.T) {
+	filler := &distinctFiller{}
+	folders := make([]orchestratorexecutor.WorkspaceFolderSpec, 2)
+	for i := range folders {
+		filler.fillStruct(t, &folders[i])
+	}
+	repos := make([]orchestratorexecutor.RepoSpec, 2)
+	for i := range repos {
+		filler.fillStruct(t, &repos[i])
+	}
+	override := &orchestratorexecutor.RouteOverride{}
+	filler.fillStruct(t, override)
+
+	launchReq := buildLifecycleLaunchRequest(&orchestratorexecutor.LaunchAgentRequest{
+		WorkspaceFolders: folders,
+		RouteOverride:    override,
+		Repositories:     repos,
+	}, "/workspace", "office-profile")
+
+	if len(launchReq.WorkspaceFolders) != len(folders) {
+		t.Fatalf("WorkspaceFolders length = %d, want %d", len(launchReq.WorkspaceFolders), len(folders))
+	}
+	for i := range folders {
+		assertSameNamedFieldsEqual(t, fmt.Sprintf("launch workspace folder %d", i), folders[i], launchReq.WorkspaceFolders[i])
+	}
+
+	if launchReq.RouteOverride == nil {
+		t.Fatal("RouteOverride was dropped from the launch request")
+	}
+	assertSameNamedFieldsEqual(t, "launch route override", *override, *launchReq.RouteOverride)
+
+	if len(launchReq.Repositories) != len(repos) {
+		t.Fatalf("Repositories length = %d, want %d", len(launchReq.Repositories), len(repos))
+	}
+	for i := range repos {
+		assertSameNamedFieldsEqual(t, fmt.Sprintf("launch repo spec %d", i), repos[i], launchReq.Repositories[i])
+	}
+}
+
 func TestBuildLifecycleLaunchRequestCarriesMCPProviders(t *testing.T) {
 	want := []string{"github", "gitlab"}
 	got := buildLifecycleLaunchRequest(&orchestratorexecutor.LaunchAgentRequest{

apps/backend/internal/gateway/websocket/access_test.go

@@ -4,15 +4,211 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 	"time"
 
+	"github.com/gin-gonic/gin"
+
 	"github.com/kandev/kandev/internal/auth/authn"
 	"github.com/kandev/kandev/internal/common/logger"
 	ws "github.com/kandev/kandev/pkg/websocket"
 	"go.uber.org/zap"
 )
 
+// connectionAuthResult captures what requireConnectionAuth did to one request:
+// whether the chain continued, the identity the downstream handler saw, and the
+// HTTP response.
+type connectionAuthResult struct {
+	recorder   *httptest.ResponseRecorder
+	nextCalled bool
+	identity   authn.Identity
+	hasID      bool
+}
+
+// runConnectionAuth drives requireConnectionAuth through a real gin router so
+// both halves of the contract are observable: c.Next() reaching the downstream
+// handler, and AbortWithStatusJSON stopping the chain before it.
+func runConnectionAuth(t *testing.T, policy AuthPolicy, query string, preset *authn.Identity) connectionAuthResult {
+	t.Helper()
+	log, err := logger.NewFromZap(zap.NewNop())
+	if err != nil {
+		t.Fatalf("logger: %v", err)
+	}
+	gateway := NewGateway(log)
+	gateway.SetAuthPolicy(policy)
+
+	result := connectionAuthResult{recorder: httptest.NewRecorder()}
+	router := gin.New()
+	if preset != nil {
+		identity := *preset
+		router.Use(func(c *gin.Context) {
+			authn.SetOnGin(c, identity)
+			c.Next()
+		})
+	}
+	router.GET("/ws", gateway.requireConnectionAuth(), func(c *gin.Context) {
+		result.nextCalled = true
+		result.identity, result.hasID = authn.FromGin(c)
+		c.Status(http.StatusOK)
+	})
+	router.ServeHTTP(result.recorder, httptest.NewRequest(http.MethodGet, "/ws"+query, nil))
+	return result
+}
+
+// TestRequireConnectionAuth covers the 401 gate in front of WebSocket upgrades
+// and the VS Code / port proxy routes.
+//
+// Must-not-regress: the enforced-and-anonymous row. Inverting the enforcement
+// check (`policy.Enforced()` instead of `!policy.Enforced()`) turns this
+// middleware into a total auth bypass precisely when auth IS enforced, and
+// nothing else in the suite notices.
+func TestRequireConnectionAuth(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+
+	const patToken = "kandev_pat_valid"
+	sessionIdentity := authn.Identity{UserID: "user-session", Role: authn.RoleMember, SessionID: "sess-1"}
+	tokenIdentity := authn.Identity{UserID: "user-pat", Role: authn.RoleMember, TokenID: "tok-1"}
+
+	enforced := func() bool { return true }
+	// resolvedToken is shared by the ResolveToken stubs below and reset at the
+	// top of every subtest, so these subtests must stay sequential — do not add
+	// t.Parallel() here without giving each case its own capture.
+	var resolvedToken string
+	resolveValid := func(_ context.Context, token string) (authn.Identity, bool) {
+		resolvedToken = token
+		return tokenIdentity, true
+	}
+	resolveReject := func(_ context.Context, token string) (authn.Identity, bool) {
+		resolvedToken = token
+		return authn.Identity{}, false
+	}
+
+	cases := []struct {
+		name       string
+		policy     AuthPolicy
+		query      string
+		preset     *authn.Identity
+		wantStatus int
+		wantNext   bool
+		wantUserID string
+		wantToken  string
+	}{
+		{
+			name:       "no enforcement hook passes through",
+			policy:     AuthPolicy{},
+			wantStatus: http.StatusOK,
+			wantNext:   true,
+		},
+		{
+			name:       "enforcement disabled passes through",
+			policy:     AuthPolicy{Enforced: func() bool { return false }, ResolveToken: resolveValid},
+			wantStatus: http.StatusOK,
+			wantNext:   true,
+		},
+		{
+			name:       "enforced without identity or token is rejected",
+			policy:     AuthPolicy{Enforced: enforced, ResolveToken: resolveValid},
+			wantStatus: http.StatusUnauthorized,
+			wantNext:   false,
+		},
+		{
+			name:       "enforced with an already-resolved identity passes through",
+			policy:     AuthPolicy{Enforced: enforced},
+			preset:     &sessionIdentity,
+			wantStatus: http.StatusOK,
+			wantNext:   true,
+			wantUserID: sessionIdentity.UserID,
+		},
+		{
+			// Branch order matters: an established identity must win over a
+			// query credential. Resolving ?token= first would let any caller
+			// overwrite the identity the HTTP auth middleware already
+			// authenticated — wantToken "" pins that ResolveToken never runs.
+			name:       "enforced with an identity present ignores the query token",
+			policy:     AuthPolicy{Enforced: enforced, ResolveToken: resolveValid},
+			query:      "?token=" + patToken,
+			preset:     &sessionIdentity,
+			wantStatus: http.StatusOK,
+			wantNext:   true,
+			wantUserID: sessionIdentity.UserID,
+			wantToken:  "",
+		},
+		{
+			name:       "enforced with a valid query token passes through and sets the identity",
+			policy:     AuthPolicy{Enforced: enforced, ResolveToken: resolveValid},
+			query:      "?token=" + patToken,
+			wantStatus: http.StatusOK,
+			wantNext:   true,
+			wantUserID: tokenIdentity.UserID,
+			wantToken:  patToken,
+		},
+		{
+			name:       "enforced with a rejected query token is rejected",
+			policy:     AuthPolicy{Enforced: enforced, ResolveToken: resolveReject},
+			query:      "?token=" + patToken,
+			wantStatus: http.StatusUnauthorized,
+			wantNext:   false,
+			wantToken:  patToken,
+		},
+		{
+			name:       "enforced with a query token but no resolver is rejected",
+			policy:     AuthPolicy{Enforced: enforced},
+			query:      "?token=" + patToken,
+			wantStatus: http.StatusUnauthorized,
+			wantNext:   false,
+		},
+		{
+			name:       "enforced with an empty query token is rejected",
+			policy:     AuthPolicy{Enforced: enforced, ResolveToken: resolveValid},
+			query:      "?token=",
+			wantStatus: http.StatusUnauthorized,
+			wantNext:   false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			resolvedToken = ""
+			got := runConnectionAuth(t, tc.policy, tc.query, tc.preset)
+
+			if got.recorder.Code != tc.wantStatus {
+				t.Errorf("status = %d, want %d (body %s)", got.recorder.Code, tc.wantStatus, got.recorder.Body.String())
+			}
+			if got.nextCalled != tc.wantNext {
+				t.Errorf("downstream handler called = %v, want %v", got.nextCalled, tc.wantNext)
+			}
+			if resolvedToken != tc.wantToken {
+				t.Errorf("ResolveToken received %q, want %q", resolvedToken, tc.wantToken)
+			}
+			if tc.wantStatus == http.StatusUnauthorized {
+				assertAuthRequiredBody(t, got.recorder)
+				return
+			}
+			if tc.wantUserID != "" {
+				if !got.hasID {
+					t.Fatalf("downstream handler saw no identity, want user %q", tc.wantUserID)
+				}
+				if got.identity.UserID != tc.wantUserID {
+					t.Errorf("identity user = %q, want %q", got.identity.UserID, tc.wantUserID)
+				}
+			}
+		})
+	}
+}
+
+func assertAuthRequiredBody(t *testing.T, rec *httptest.ResponseRecorder) {
+	t.Helper()
+	var body map[string]string
+	if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+		t.Fatalf("401 body is not JSON (%q): %v", rec.Body.String(), err)
+	}
+	if body["error"] != "authentication required" {
+		t.Errorf("401 body error = %q, want %q", body["error"], "authentication required")
+	}
+}
+
 func newAccessTestHub(t *testing.T) *Hub {
 	t.Helper()
 	log, err := logger.NewFromZap(zap.NewNop())

apps/backend/internal/worktree/config_test.go

@@ -2,6 +2,54 @@ package worktree
 
 import "testing"
 
+// TestConfigValidateBranchPrefix pins both halves of the default-application
+// branch in Config.Validate: an empty prefix picks up DefaultBranchPrefix, and
+// an explicitly configured prefix survives untouched. Inverting the condition
+// (`!= ""`) breaks both and must fail here.
+func TestConfigValidateBranchPrefix(t *testing.T) {
+	cases := []struct {
+		name  string
+		given string
+		want  string
+	}{
+		{name: "empty prefix falls back to the default", given: "", want: DefaultBranchPrefix},
+		{name: "explicit prefix is preserved", given: "kandev/", want: "kandev/"},
+		{name: "explicit prefix without separator is preserved", given: "wip", want: "wip"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := &Config{BranchPrefix: tc.given}
+			if err := cfg.Validate(); err != nil {
+				t.Fatalf("Validate() error = %v, want nil", err)
+			}
+			if cfg.BranchPrefix != tc.want {
+				t.Errorf("BranchPrefix = %q, want %q", cfg.BranchPrefix, tc.want)
+			}
+		})
+	}
+}
+
+// TestConfigValidateLeavesOtherFieldsAlone guards against Validate growing a
+// side effect on fields it has no business defaulting.
+func TestConfigValidateLeavesOtherFieldsAlone(t *testing.T) {
+	cfg := &Config{
+		Enabled:             true,
+		TasksBasePath:       "/tmp/kandev-tasks",
+		BranchPrefix:        "feat/",
+		FetchTimeoutSeconds: 11,
+		PullTimeoutSeconds:  22,
+	}
+	want := *cfg
+
+	if err := cfg.Validate(); err != nil {
+		t.Fatalf("Validate() error = %v, want nil", err)
+	}
+	if *cfg != want {
+		t.Errorf("Validate() mutated the config: got %+v, want %+v", *cfg, want)
+	}
+}
+
 func TestTaskDirSuffix(t *testing.T) {
 	const id = "61ccfd2c-1121-4226-99ab-8d9a60a57e6e"