package channel

import (
	"context"
	"errors"
	"fmt"
	"net/http"
	"net/http/httptest"
	"net/url"
	"sync"
	"testing"

	"github.com/gin-gonic/gin"
	"github.com/stretchr/testify/require"

	"github.com/tidbcloud/AgentTeam/internal/api"
	"github.com/tidbcloud/AgentTeam/internal/api/daemon"
	"github.com/tidbcloud/AgentTeam/internal/api/dto"
	"github.com/tidbcloud/AgentTeam/internal/runtimecontrol"
	"github.com/tidbcloud/AgentTeam/internal/store/agent"
)

func TestReviewHubErrorMapsTargetPathUpgrade(t *testing.T) {
	t.Parallel()

	err := reviewHubError(runtimecontrol.ErrDaemonWorkspaceTargetPathUpgradeRequired)

	var httpErr api.HTTPError
	require.True(t, errors.As(err, &httpErr))
	require.Equal(t, http.StatusConflict, httpErr.Status)
	require.Equal(t, runtimecontrol.ErrDaemonWorkspaceTargetPathUpgradeRequired.Error(), httpErr.Error())
}

func TestReviewHubErrorMapsFingerprintUnavailable(t *testing.T) {
	t.Parallel()

	err := reviewHubError(runtimecontrol.ErrDaemonReviewFingerprintUnavailable)

	var httpErr api.HTTPError
	require.True(t, errors.As(err, &httpErr))
	require.Equal(t, http.StatusNotImplemented, httpErr.Status)
	require.Equal(t, runtimecontrol.ReviewFingerprintDaemonUpgradeMessage, httpErr.Error())
}

// TestReviewHubErrorMapsTargetUnsupportedToHTTP422 pins the channel HTTP
// layer's mapping for the unified non-fallback sentinel. When the Hub-side
// mapper reconstructs ErrReviewTargetUnsupported from the desktop's stable
// wire outcome (ReviewTargetUnsupportedOutcomeCode) or legacy ErrorCode
// (ReviewTargetUnsupportedLegacyErrorCode), this branch is what returns it
// as a controlled 422 instead of a generic 500.
//
// Why 422 and not 501: this is NOT a capability miss (desktop-too-old). The
// desktop DID answer — it signaled a product limitation (control-char paths
// in the worktree) that no daemon upgrade can fix. 501 would mislead the
// frontend into prompting for an upgrade; 422 tells it the repo itself is
// the problem. See the elastic/session carve-out tests in
// internal/api/daemon/remote_endpoint_unsupported_classification_test.go
// for the upstream half of this contract.
//
// Mutation contract: remove the errors.Is(err, ErrReviewTargetUnsupported)
// branch in reviewHubError and this test fails — the fall-through returns
// HTTP 500.
func TestReviewHubErrorMapsTargetUnsupportedToHTTP422(t *testing.T) {
	t.Parallel()

	err := reviewHubError(runtimecontrol.ErrReviewTargetUnsupported)

	var httpErr api.HTTPError
	require.True(t, errors.As(err, &httpErr))
	require.Equal(t, http.StatusUnprocessableEntity, httpErr.Status)
	require.Equal(t, runtimecontrol.ReviewTargetUnsupportedMessage, httpErr.Error())
}

// TestReviewHubErrorMapsStableBusyToHTTP503 pins the second half of the
// Explore/Thread classification-parity contract: when the remote-endpoint
// routing layer emits a stable business error for an ElasticPreferred
// snapshot/fingerprint request (ERL saturated, emergency disabled, dial
// timeout, etc. — and the session data plane is also unavailable), the
// channel endpoint MUST map it to HTTP 503, mirroring the Explore entry.
//
// Without this mapping, a transient ERL capacity hit would return as an
// frontend as a generic 500, defeating the whole point of the stable-error
// classification and breaking the cadence/polling agentteam. See
// reviewHubStableErrorClassification in api_thread_review.go and the
// symmetric remoteRequestStableErrorClassification in webagent/api_review.go.
func TestReviewHubErrorMapsStableBusyToHTTP503(t *testing.T) {
	t.Parallel()

	cases := []struct {
		name     string
		err      error
		expected string
	}{
		{
			name:     "desktop_busy",
			err:      errors.New("desktop_busy"),
			expected: "Desktop is busy",
		},
		{
			name:     "desktop_unavailable",
			err:      errors.New("desktop_unavailable"),
			expected: "Desktop endpoint is unavailable",
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()

			err := reviewHubError(tc.err)

			var httpErr api.HTTPError
			require.True(t, errors.As(err, &httpErr))
			require.Equal(t, http.StatusServiceUnavailable, httpErr.Status,
				"stable business errors MUST return as 503 so clients retry; got %v", err)
			require.Equal(t, tc.expected, httpErr.Error())
		})
	}
}

type threadReviewStoreStub struct {
	rc       *agent.ReviewContext
	contexts []agent.ReviewContext
}

func (store threadReviewStoreStub) GetReviewContext(_ context.Context, _ string, _ string) (*agent.ReviewContext, error) {
	return store.rc, nil
}

func (store threadReviewStoreStub) ListReviewContextsForThread(_ context.Context, _ string, _ string, _ string) ([]agent.ReviewContext, error) {
	if len(store.contexts) > 0 {
		return store.contexts, nil
	}
	if store.rc == nil {
		return nil, nil
	}
	return []agent.ReviewContext{*store.rc}, nil
}

type threadReviewHubStub struct {
	mu              sync.Mutex
	lastRequest     runtimecontrol.RemoteRequestContext
	summary         agent.AgentReviewSummary
	files           map[string][]agent.AgentReviewFile
	fileCalls       []string
	snapshot        *agent.AgentReviewSnapshot
	fingerprint     *agent.AgentReviewFingerprint
	fingerprintErr  error
	fingerprintN    int
	lastDiffRequest threadReviewDiffRequest
	batchDiffs      map[string]agent.AgentReviewDiff
	batchErrByPath  map[string]error
	batchErr        error
	batchCalls      int
	lastBatchArgs   threadReviewBatchArgs
}

// threadReviewDiffRequest captures the arguments passed to
// GetAgentReviewDiff so path-precision tests can assert the raw bytes
// survived the handler without TrimSpace or normalization.
type threadReviewDiffRequest struct {
	scope      string
	path       string
	baseRef    string
	headRef    string
	comparison string
}

// threadReviewBatchArgs captures the arguments passed to
// GetAgentReviewDiffBatch so the batch handler tests can assert scope, refs,
// and the raw path slice survived the handler verbatim.
type threadReviewBatchArgs struct {
	targetPath string
	scope      string
	paths      []string
	baseRef    string
	headRef    string
	comparison string
}

func (hub *threadReviewHubStub) GetAgentReviewSummary(ctx context.Context, _ string, _ string, _ string, _ string, _ string, _ string) (agent.AgentReviewSummary, error) {
	hub.lastRequest = runtimecontrol.RemoteRequestContextFromContext(ctx)
	return hub.summary, nil
}

func (hub *threadReviewHubStub) GetAgentReviewSnapshot(ctx context.Context, _ string, _ string, _ string, _ string, _ string, _ string) (agent.AgentReviewSnapshot, error) {
	hub.lastRequest = runtimecontrol.RemoteRequestContextFromContext(ctx)
	if hub.snapshot == nil {
		return agent.AgentReviewSnapshot{}, runtimecontrol.ErrDaemonReviewSnapshotUnavailable
	}
	return *hub.snapshot, nil
}

func (hub *threadReviewHubStub) GetAgentReviewFingerprint(ctx context.Context, _ string, _ string, _ string, _ string, _ string, _ string) (agent.AgentReviewFingerprint, error) {
	hub.lastRequest = runtimecontrol.RemoteRequestContextFromContext(ctx)
	hub.fingerprintN++
	// Explicit typed error injection takes priority over the capability-miss
	// default. This lets tests drive the typed-sentinel → HTTP mapping (e.g.
	// ErrReviewTargetUnsupported → 422) without conflating unsupported outcome
	// with the desktop-too-old 501 case.
	if hub.fingerprintErr != nil {
		return agent.AgentReviewFingerprint{}, hub.fingerprintErr
	}
	if hub.fingerprint == nil {
		return agent.AgentReviewFingerprint{}, runtimecontrol.ErrDaemonReviewFingerprintUnavailable
	}
	return *hub.fingerprint, nil
}
func (hub *threadReviewHubStub) ListAgentReviewFiles(_ context.Context, _, _, _, scope, _, _, _ string) ([]agent.AgentReviewFile, error) {
	hub.mu.Lock()
	defer hub.mu.Unlock()
	hub.fileCalls = append(hub.fileCalls, scope)
	return hub.files[scope], nil
}

func (hub *threadReviewHubStub) GetAgentReviewDiff(_ context.Context, _, _, _, scope, path, baseRef, headRef, comparison string) (agent.AgentReviewDiff, error) {
	hub.mu.Lock()
	defer hub.mu.Unlock()
	hub.lastDiffRequest = threadReviewDiffRequest{
		scope:      scope,
		path:       path,
		baseRef:    baseRef,
		headRef:    headRef,
		comparison: comparison,
	}
	return agent.AgentReviewDiff{}, nil
}

func (hub *threadReviewHubStub) GetAgentReviewDiffBatch(_ context.Context, _, _, targetPath, scope string, paths []string, baseRef, headRef, comparison string) (map[string]daemon.AgentReviewDiffBatchResult, error) {
	hub.mu.Lock()
	defer hub.mu.Unlock()
	hub.batchCalls++
	hub.lastBatchArgs = threadReviewBatchArgs{
		targetPath: targetPath,
		scope:      scope,
		paths:      append([]string(nil), paths...),
		baseRef:    baseRef,
		headRef:    headRef,
		comparison: comparison,
	}
	if hub.batchErr != nil {
		return nil, hub.batchErr
	}
	results := make(map[string]daemon.AgentReviewDiffBatchResult, len(paths))
	for _, path := range paths {
		if err, ok := hub.batchErrByPath[path]; ok {
			results[path] = daemon.AgentReviewDiffBatchResult{Error: err}
			continue
		}
		if diff, ok := hub.batchDiffs[path]; ok {
			results[path] = daemon.AgentReviewDiffBatchResult{Diff: diff}
			continue
		}
		results[path] = daemon.AgentReviewDiffBatchResult{}
	}
	return results, nil
}

func (hub *threadReviewHubStub) ListAgentOutputTargetFiles(context.Context, string, string, string, string) ([]agent.WorkspaceFile, error) {
	return nil, nil
}

func (hub *threadReviewHubStub) ListAgentOutputTargetFilesPage(context.Context, string, string, string, string, agent.WorkspaceListOptions) (agent.WorkspaceFilePage, error) {
	return agent.WorkspaceFilePage{}, nil
}

func (hub *threadReviewHubStub) ReadAgentOutputTargetFile(context.Context, string, string, string, string) (agent.WorkspaceFileContent, error) {
	return agent.WorkspaceFileContent{}, nil
}

func TestThreadReviewSummaryMarksRemoteRequestUserInteractive(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{summary: agent.AgentReviewSummary{Status: "ready"}}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	_, err := handler.ReviewSummary(req)
	require.NoError(t, err)
	require.Equal(t, runtimecontrol.RemoteRequestOriginUserInteractive, hub.lastRequest.Origin)
	require.Equal(t, runtimecontrol.RemoteRequestCauseWebAPI, hub.lastRequest.Cause)
}

func TestThreadReviewSnapshotUsesSummaryEmbeddedFiles(t *testing.T) {
	t.Parallel()

	branchFiles := []agent.AgentReviewFile{{Path: "branch.go"}}
	workingFiles := []agent.AgentReviewFile{{Path: "working.go", Unstaged: true}}
	hub := &threadReviewHubStub{snapshot: &agent.AgentReviewSnapshot{
		Summary:      agent.AgentReviewSummary{Status: "ready", BranchFileCount: 1, UnstagedFileCount: 1},
		Files:        branchFiles,
		WorkingFiles: workingFiles,
	}}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	response, err := handler.ReviewSnapshot(req)
	require.NoError(t, err)
	require.Empty(t, hub.fileCalls)
	require.Equal(t, http.StatusOK, response.Status)
}

func TestThreadReviewSnapshotFallsBackForOlderDaemon(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{
		summary: agent.AgentReviewSummary{
			Status:            "ready",
			BranchFileCount:   1,
			UnstagedFileCount: 1,
		},
		files: map[string][]agent.AgentReviewFile{
			"branch":  {{Path: "branch.go"}},
			"working": {{Path: "working.go", Unstaged: true}},
		},
	}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	_, err := handler.ReviewSnapshot(req)
	require.NoError(t, err)
	require.ElementsMatch(t, []string{"branch", "working"}, hub.fileCalls)
}

// TestThreadReviewFingerprintReturnsPayloadOnCapabilityHit proves the
// happy path: capability present → 200 with the fingerprint payload, and
// the stub's fingerprint pointer is forwarded unchanged.
func TestThreadReviewFingerprintReturnsPayloadOnCapabilityHit(t *testing.T) {
	t.Parallel()

	want := agent.AgentReviewFingerprint{
		HeadCommit:      "abc123",
		RepositoryToken: "repo-token",
		BranchToken:     "branch-token",
		WorkingToken:    "working-token",
		WorkingStrength: agent.AgentReviewFingerprintWorkingStrengthHint,
	}
	hub := &threadReviewHubStub{fingerprint: &want}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	response, err := handler.ReviewFingerprint(req)
	require.NoError(t, err)
	require.Equal(t, http.StatusOK, response.Status)
	require.Equal(t, runtimecontrol.RemoteRequestOriginUserInteractive, hub.lastRequest.Origin)
	require.Equal(t, 1, hub.fingerprintN, "fingerprint RPC must be called exactly once")
}

// TestThreadReviewFingerprintReturnsCapabilityMissAs501 proves the
// capability-miss contract: legacy daemon → ReviewFingerprint returns
// HTTP 501 with the upgrade message so the frontend can fall back to
// polling snapshot at the slower cadence. No fake fingerprint is
// synthesised on the Hub side.
func TestThreadReviewFingerprintReturnsCapabilityMissAs501(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{fingerprint: nil} // nil → capability miss
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	_, err := handler.ReviewFingerprint(req)
	var httpErr api.HTTPError
	require.True(t, errors.As(err, &httpErr))
	require.Equal(t, http.StatusNotImplemented, httpErr.Status)
	require.Equal(t, runtimecontrol.ReviewFingerprintDaemonUpgradeMessage, httpErr.Error())
}

// TestThreadReviewFingerprintMapsUnsupportedOutcomeToHTTP422 pins the
// handler-level end-to-end: Hub returns ErrReviewTargetUnsupported (which
// is what the production mapper produces when the desktop emits the stable
// ReviewTargetUnsupportedOutcomeCode on the wire), and the channel
// ReviewFingerprint handler MUST target HTTP 422 — NOT 500 (generic) and
// NOT 501 (capability miss). This test plus the daemon-side outcome→sentinel
// tests together prove the full wire-outcome → HTTP-422 chain.
//
// The stub injects the typed sentinel directly because the outcome→sentinel
// mapping lives in the daemon Hub (covered by the elastic/session
// classification tests). Wiring a full Hub here would duplicate that
// coverage and couple this test to daemon internals.
func TestThreadReviewFingerprintMapsUnsupportedOutcomeToHTTP422(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{fingerprintErr: runtimecontrol.ErrReviewTargetUnsupported}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}
	req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
	req.Gin.Request = httptest.NewRequest(http.MethodGet, "/?contextId=rcl_1", nil)

	_, err := handler.ReviewFingerprint(req)
	var httpErr api.HTTPError
	require.True(t, errors.As(err, &httpErr))
	require.Equal(t, http.StatusUnprocessableEntity, httpErr.Status,
		"unsupported repo content MUST target as 422 so the frontend distinguishes it from transient 500s and the capability-miss 501; got %v", err)
	require.Equal(t, runtimecontrol.ReviewTargetUnsupportedMessage, httpErr.Error())
	require.Equal(t, 1, hub.fingerprintN, "fingerprint RPC must be called exactly once")
}

// TestThreadReviewDiffPreservesRawPathThroughHandler pins the raw-path
// discipline at the API boundary: a Git file path containing leading or
// trailing whitespace MUST reach the Hub verbatim. Any TrimSpace in the
// handler would silently rewrite " foo.go" → "foo.go" and point the diff
// at a different file.
func TestThreadReviewDiffPreservesRawPathThroughHandler(t *testing.T) {
	t.Parallel()

	cases := []struct {
		name string
		path string
	}{
		{"leading space", " foo.go"},
		{"trailing space", "foo.go "},
		{"interior space", "foo bar.go"},
		{"tab", "foo\tgo"},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()
			// Each subtest gets its own hub so parallel subtests don't
			// race on lastDiffRequest.
			hub := &threadReviewHubStub{}
			handler := &ThreadHandler{
				reviewStore: threadReviewStoreStub{
					rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
				},
				hub: hub,
			}
			req := channelWorkspaceRequest(gin.Param{Key: "threadChannelId", Value: "thr_1"})
			// url.PathEscape preserves the literal bytes; the Gin query
			// decoder gives us those exact bytes back.
			req.Gin.Request = httptest.NewRequest(http.MethodGet,
				"/?contextId=rcl_1&scope=branch&path="+url.PathEscape(tc.path), nil)

			_, err := handler.ReviewDiff(req)
			require.NoError(t, err)
			require.Equal(t, tc.path, hub.lastDiffRequest.path,
				"path must reach the Hub verbatim — no TrimSpace or normalization at the handler")
		})
	}
}

// TestThreadReviewDiffBatchPerservesRawPathsAndBuildsResponse is the
// channel-side pin for the batch endpoint. It proves:
//
//  1. Every requested path is forwarded to the Hub in a single
//     GetAgentReviewDiffBatch call (no handler-level fan-out).
//  2. Per-path diff payloads from the Hub appear in the response body's
//     diffs map, keyed by the exact raw path bytes the request carried.
//  3. Per-path errors (e.g. a path the Hub couldn't produce) are acked
//     in the response body's errors map — NOT as an HTTP error — so a
//     single bad path does not poison the batch.
//  4. Top-level Hub errors (capability miss, machine offline) still map to
//     the normal reviewHubError HTTP statuses (501/503/etc).
func TestThreadReviewDiffBatchPerservesRawPathsAndBuildsResponse(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{
		batchDiffs: map[string]agent.AgentReviewDiff{
			"foo.go":  {Status: "modified", Path: "foo.go"},
			" bar.go": {Status: "added", Path: " bar.go"},
		},
		batchErrByPath: map[string]error{
			// "missing.go" models a per-path failure (e.g. transient I/O on
			// the desktop side). The Hub returns a typed error for THIS path
			// only — the other two paths succeed, proving per-path errors
			// do not poison the batch.
			"missing.go": errors.New("transient desktop I/O"),
		},
	}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}

	body := dto.ReviewDiffBatchRequest{
		Scope: "branch",
		Paths: []string{"foo.go", " bar.go", "missing.go"},
	}
	req := channelWorkspaceJSONWithParams(body, gin.Param{Key: "threadChannelId", Value: "thr_1"})

	response, err := handler.ReviewDiffBatch(req)
	require.NoError(t, err)
	require.Equal(t, http.StatusOK, response.Status)
	require.Equal(t, 1, hub.batchCalls, "handler MUST fan-in to exactly one Hub batch call")
	require.Equal(t, []string{"foo.go", " bar.go", "missing.go"}, hub.lastBatchArgs.paths,
		"raw path bytes — including leading/trailing whitespace — MUST survive the handler verbatim")

	typed, ok := response.Body.(dto.ReviewDiffBatchResponse)
	require.True(t, ok, "response body MUST be a ReviewDiffBatchResponse, got %T", response.Body)
	require.Len(t, typed.Diffs, 2)
	require.Equal(t, "modified", typed.Diffs["foo.go"].Status)
	require.Equal(t, "added", typed.Diffs[" bar.go"].Status)
	require.Contains(t, typed.Errors, "missing.go",
		"paths the Hub could not produce MUST target in the errors map")
}

// TestThreadReviewDiffBatchRejectsEmptyPathEntries pins the input
// validation: an empty string in the paths slice is a client bug and
// MUST target as 400, not silently reach the Hub.
func TestThreadReviewDiffBatchRejectsEmptyPathEntries(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}

	body := dto.ReviewDiffBatchRequest{Scope: "branch", Paths: []string{"foo.go", ""}}
	req := channelWorkspaceJSONWithParams(body, gin.Param{Key: "threadChannelId", Value: "thr_1"})

	_, err := handler.ReviewDiffBatch(req)
	requireHTTPError(t, err, http.StatusBadRequest, "paths must not contain empty strings")
	require.Zero(t, hub.batchCalls, "validation MUST short-circuit before the Hub is invoked")
}

// TestThreadReviewDiffBatchCapsPathList pins the bounded-batch contract:
// the server rejects oversized batches so a misbehaving client cannot
// exhaust desktop-side memory or block the wire with thousands of paths.
func TestThreadReviewDiffBatchCapsPathList(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}

	tooMany := make([]string, 65)
	for i := range tooMany {
		tooMany[i] = fmt.Sprintf("path_%d.go", i)
	}
	body := dto.ReviewDiffBatchRequest{Scope: "branch", Paths: tooMany}
	req := channelWorkspaceJSONWithParams(body, gin.Param{Key: "threadChannelId", Value: "thr_1"})

	_, err := handler.ReviewDiffBatch(req)
	requireHTTPError(t, err, http.StatusBadRequest, "too many paths in a single batch")
	require.Zero(t, hub.batchCalls)
}

// TestThreadReviewDiffBatchReturnsUnsupportedSentinelAs422 proves the
// top-level unsupported-sentinel path: when the Hub returns
// ErrReviewTargetUnsupported for the whole batch (because the repo tree
// contains a control-char path), the handler MUST target 422 — NOT 500,
// NOT 200-with-errors. The frontend keys off 422 to show the user a stable
// product-limitation message and stop polling.
func TestThreadReviewDiffBatchReturnsUnsupportedSentinelAs422(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{batchErr: runtimecontrol.ErrReviewTargetUnsupported}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			rc: &agent.ReviewContext{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo", SourceAgentID: "agt_1"},
		},
		hub: hub,
	}

	body := dto.ReviewDiffBatchRequest{Scope: "branch", Paths: []string{"foo.go"}}
	req := channelWorkspaceJSONWithParams(body, gin.Param{Key: "threadChannelId", Value: "thr_1"})

	_, err := handler.ReviewDiffBatch(req)
	requireHTTPError(t, err, http.StatusUnprocessableEntity, runtimecontrol.ReviewTargetUnsupportedMessage)
}

// TestThreadReviewDiffBatchPicksBodyContextID pins the multi-context case:
// a reply thread can have several review contexts bound to it (one per
// shared PR, for example), and the batch endpoint carries contextId in the
// JSON body — not the query string. The handler MUST honor r.Body.ContextID
// and select the matching context; otherwise it would silently fall back to
// "first context wins" and load the diff against the wrong review.
func TestThreadReviewDiffBatchPicksBodyContextID(t *testing.T) {
	t.Parallel()

	hub := &threadReviewHubStub{
		batchDiffs: map[string]agent.AgentReviewDiff{
			"foo.go": {Status: "modified", Path: "foo.go"},
		},
	}
	handler := &ThreadHandler{
		reviewStore: threadReviewStoreStub{
			contexts: []agent.ReviewContext{
				{ID: "rcl_1", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo-alpha", SourceAgentID: "agt_1"},
				{ID: "rcl_2", ThreadID: "thr_1", MachineID: "mach_1", RootPath: "/repo-beta", SourceAgentID: "agt_2"},
			},
		},
		hub: hub,
	}

	body := dto.ReviewDiffBatchRequest{
		Scope:     "branch",
		ContextID: "rcl_2",
		Paths:     []string{"foo.go"},
	}
	req := channelWorkspaceJSONWithParams(body, gin.Param{Key: "threadChannelId", Value: "thr_1"})

	response, err := handler.ReviewDiffBatch(req)
	require.NoError(t, err)
	require.Equal(t, http.StatusOK, response.Status)
	require.Equal(t, 1, hub.batchCalls)
	require.Equal(t, "/repo-beta", hub.lastBatchArgs.targetPath,
		"body.ContextID MUST drive context selection so the second bound context wins")
}