package skill

import (
	"context"
	"errors"
	"os"
	"path/filepath"
	"strings"
	stdlibsync "sync"
	"sync/atomic"
	"testing"

	gittransport "github.com/go-git/go-git/v5/plumbing/transport/http"
	"github.com/stretchr/testify/require"
	"github.com/tidbcloud/AgentTeam/desktop/internal/dispatcher"
	"github.com/tidbcloud/AgentTeam/desktop/internal/profile"
)

// fakeGit is an in-memory GitRunner for the sync producer. It records each
// call and never touches the network or real git.
type fakeGit struct {
	cloneCalls  []fakeCloneCall
	fetchCalls  []fakeFetchCall
	cloneRev    string
	fetchRev    string
	cloneErr    error
	fetchErr    error
	cloneStubFn func(url, branch, dest string) error
}

type fakeCloneCall struct {
	URL, Branch, Dest string
	Auth              gittransport.AuthMethod
}
type fakeFetchCall struct {
	RepoDir, Branch string
	Auth            gittransport.AuthMethod
}

func (f *fakeGit) Clone(ctx context.Context, url, branch, dest string, auth gittransport.AuthMethod) (string, error) {
	f.cloneCalls = append(f.cloneCalls, fakeCloneCall{url, branch, dest, auth})
	if f.cloneErr != nil {
		return "", f.cloneErr
	}
	if f.cloneStubFn != nil {
		if err := f.cloneStubFn(url, branch, dest); err != nil {
			return "", err
		}
	}
	return f.cloneRev, nil
}

func (f *fakeGit) FetchCheckout(ctx context.Context, repoDir, branch string, auth gittransport.AuthMethod) (string, error) {
	f.fetchCalls = append(f.fetchCalls, fakeFetchCall{repoDir, branch, auth})
	if f.fetchErr != nil {
		return "", f.fetchErr
	}
	return f.fetchRev, nil
}

func (f *fakeGit) RemoteURL(repoDir string) (string, error) {
	data, err := os.ReadFile(filepath.Join(repoDir, ".git", "config"))
	if err != nil {
		return "", err
	}
	for _, line := range strings.Split(string(data), "\n") {
		line = strings.TrimSpace(line)
		if strings.HasPrefix(line, "url =") {
			return strings.TrimSpace(strings.TrimPrefix(line, "url =")), nil
		}
	}
	return "", errors.New("origin remote url not found")
}

// stubProvider returns a canned access list with a LookupState, ignoring
// ctx/agentID. Tests pick Bound/Unbound/Unknown to drive each code path.
type stubProvider struct {
	access []SkillAccess
	state  dispatcher.SkillLookupState
	err    error
}

func (s stubProvider) Lookup(_ context.Context, _ string) ([]SkillAccess, dispatcher.SkillLookupState, error) {
	if s.err != nil {
		return nil, dispatcher.SkillLookupUnknown, s.err
	}
	if s.state == dispatcher.SkillLookupBound && len(s.access) == 0 {
		// Defensive: provider declared Bound but returned no entries. Sync
		// treats this as Unknown; mirror that in the stub so tests stay honest.
		return nil, dispatcher.SkillLookupUnknown, nil
	}
	return s.access, s.state, nil
}

// newTestSync wires a Sync with the fake git runner and a fresh profile
// manager rooted at t.TempDir().
func newTestSync(t *testing.T, git GitRunner, provider ProviderFunc) (*Sync, *profile.ManagedSkillRepositoryManager) {
	t.Helper()
	mgr := profile.New(t.TempDir()).ManagedSkillRepositories()
	return NewSync(provider, git, mgr), mgr
}

// createFakeCloneOnDisk materializes a minimal repository dir on disk so the
// "fetch path" branch (record exists with LocalPath) can run.
func createFakeCloneOnDisk(t *testing.T, mgr *profile.ManagedSkillRepositoryManager, sk SkillAccess, revision string) {
	t.Helper()
	repoPath := mgr.RepositoryPath(sk.RepoID)
	require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "skill-a"), 0o755))
	require.NoError(t, os.WriteFile(filepath.Join(repoPath, "skill-a", "SKILL.md"),
		[]byte("# A\n"), 0o644))
	writeFakeRemoteConfig(t, repoPath, sk.CloneURL)
	// Pre-upsert the record so the producer sees LocalPath set.
	require.NoError(t, mgr.Upsert(profile.ManagedSkillRepositoryRecord{
		RepoID:    sk.RepoID,
		Name:      sk.Name,
		CloneURL:  sk.CloneURL,
		Branch:    sk.Branch,
		Revision:  revision,
		LocalPath: repoPath,
	}))
}

func writeFakeRemoteConfig(t *testing.T, repoPath, remoteURL string) {
	t.Helper()
	require.NoError(t, os.MkdirAll(filepath.Join(repoPath, ".git"), 0o755))
	require.NoError(t, os.WriteFile(filepath.Join(repoPath, ".git", "config"), []byte(`[remote "origin"]
	url = `+remoteURL+`
`), 0o644))
}

// boundProvider is a shorthand for the common happy-path stub.
func boundProvider(access []SkillAccess) stubProvider {
	return stubProvider{access: access, state: dispatcher.SkillLookupBound}
}

func TestSyncAgent_ClonesAndUpsertsRecord(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_abc",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)

	// Materialize the clone dir so countSkills can see it (real git would
	// have populated it; the fake just records the call).
	git.cloneStubFn = func(_, _, dest string) error {
		require.NoError(t, os.MkdirAll(filepath.Join(dest, "skill-a"), 0o755))
		return os.WriteFile(filepath.Join(dest, "skill-a", "SKILL.md"), []byte("# a\n"), 0o644)
	}

	paths, state, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, state)
	require.Len(t, paths, 1)

	expectedPath := mgr.RepositoryPath("sk_abc")
	require.Equal(t, expectedPath, paths[0])
	require.Len(t, git.cloneCalls, 1)
	require.Equal(t, "main", git.cloneCalls[0].Branch)
	require.Equal(t, "https://agentteam.example.com/agentteam/ws-1.sk.git", git.cloneCalls[0].URL)
	require.Equal(t, expectedPath, git.cloneCalls[0].Dest)
	require.Empty(t, git.fetchCalls, "first sync should clone, not fetch")

	rec, ok := mgr.Get("sk_abc")
	require.True(t, ok)
	require.Equal(t, "rev_abc", rec.Revision)
	require.Equal(t, "main", rec.Branch)
	require.Equal(t, expectedPath, rec.LocalPath)
	require.Equal(t, 1, rec.SkillCount)
	require.Equal(t, profile.SkillSyncStateReady, rec.LastSyncState)
	require.False(t, rec.LastSyncAt.IsZero())
	require.False(t, rec.LastAttemptAt.IsZero())
}

func TestSyncAgent_FetchesWhenCloneAlreadyOnDisk(t *testing.T) {
	git := &fakeGit{fetchRev: "rev_def"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_abc",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	createFakeCloneOnDisk(t, mgr, sk, "rev_abc")

	paths, state, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, state)
	require.Len(t, paths, 1)
	require.Empty(t, git.cloneCalls, "existing clone on disk should not be re-cloned")
	require.Len(t, git.fetchCalls, 1)
	require.Equal(t, "main", git.fetchCalls[0].Branch)
	require.Equal(t, mgr.RepositoryPath("sk_abc"), git.fetchCalls[0].RepoDir)

	rec, ok := mgr.Get("sk_abc")
	require.True(t, ok)
	require.Equal(t, "rev_def", rec.Revision, "revision must reflect fetched HEAD")
}

func TestSyncAgent_ReclonesWhenExistingRemoteDoesNotMatchAccess(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_new"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/user/current.sk.git",
		RepoID:   "sk_abc",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	createFakeCloneOnDisk(t, mgr, sk, "rev_old")
	repoPath := mgr.RepositoryPath("sk_abc")
	writeFakeRemoteConfig(t, repoPath, "https://agentteam.example.com/workspace/stale.sk.git")
	staleFile := filepath.Join(repoPath, "stale-only.txt")
	require.NoError(t, os.WriteFile(staleFile, []byte("old cache"), 0o644))
	git.cloneStubFn = func(_, _, dest string) error {
		require.NoFileExists(t, staleFile, "stale clone must be removed before re-clone")
		require.NoError(t, os.MkdirAll(filepath.Join(dest, "skill-new"), 0o755))
		writeFakeRemoteConfig(t, dest, sk.CloneURL)
		return os.WriteFile(filepath.Join(dest, "skill-new", "SKILL.md"), []byte("# new\n"), 0o644)
	}

	paths, state, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, state)
	require.Equal(t, []string{repoPath}, paths)
	require.Empty(t, git.fetchCalls, "stale remote must not be fetched with the new skill token")
	require.Len(t, git.cloneCalls, 1)
	require.Equal(t, sk.CloneURL, git.cloneCalls[0].URL)
	require.Equal(t, repoPath, git.cloneCalls[0].Dest)
	require.NoFileExists(t, staleFile)

	rec, ok := mgr.Get("sk_abc")
	require.True(t, ok)
	require.Equal(t, sk.CloneURL, rec.CloneURL)
	require.Equal(t, "rev_new", rec.Revision)
	require.Equal(t, profile.SkillSyncStateReady, rec.LastSyncState)
	require.Equal(t, 1, rec.SkillCount)
}

func TestSyncAgent_ReusesRecentRepoSyncAcrossAgents(t *testing.T) {
	git := &fakeGit{fetchRev: "rev_def"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_shared",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	createFakeCloneOnDisk(t, mgr, sk, "rev_abc")

	pathsA, stateA, err := sync.SyncAgent(context.Background(), "agt_a", "agt_a@ch#https://s:ws")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, stateA)
	require.Len(t, pathsA, 1)

	pathsB, stateB, err := sync.SyncAgent(context.Background(), "agt_b", "agt_b@ch#https://s:ws")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, stateB)
	require.Equal(t, pathsA, pathsB)
	require.Empty(t, git.cloneCalls)
	require.Len(t, git.fetchCalls, 1, "shared repo must only fetch once inside the reuse window")
}

func TestSyncAgent_DoesNotReuseAfterRepoSyncFailure(t *testing.T) {
	git := &fakeGit{fetchErr: errors.New("network unreachable")}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_shared",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	createFakeCloneOnDisk(t, mgr, sk, "rev_abc")

	pathsA, stateA, err := sync.SyncAgent(context.Background(), "agt_a", "agt_a@ch#https://s:ws")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, stateA)
	require.Empty(t, pathsA)

	pathsB, stateB, err := sync.SyncAgent(context.Background(), "agt_b", "agt_b@ch#https://s:ws")
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, stateB)
	require.Empty(t, pathsB)
	require.Len(t, git.fetchCalls, 2, "failed sync must not enter the repo reuse window")
}

func TestSyncAgent_ReclonesWhenRecordExistsButLocalPathMissing(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_new"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_abc",
	}
	sync, mgr := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	// Pre-upsert a record pointing at a path that does NOT exist on disk.
	require.NoError(t, mgr.Upsert(profile.ManagedSkillRepositoryRecord{
		RepoID:    "sk_abc",
		Branch:    "main",
		CloneURL:  sk.CloneURL,
		Revision:  "rev_old",
		LocalPath: mgr.RepositoryPath("sk_abc"),
	}))

	paths, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Len(t, paths, 1)
	require.Len(t, git.cloneCalls, 1, "missing local clone should trigger re-clone")
	require.Empty(t, git.fetchCalls)
}

func TestSyncAgent_DedupesByRepoID(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_one"}
	// Same RepoID backs two skills — only one clone should happen.
	access := []SkillAccess{
		{SkillID: "skill_1", Name: "A", Branch: "main",
			CloneURL: "https://x/a.sk.git", RepoID: "sk_shared"},
		{SkillID: "skill_2", Name: "B", Branch: "main",
			CloneURL: "https://x/a.sk.git", RepoID: "sk_shared"},
	}
	sync, mgr := newTestSync(t, git, boundProvider(access).Lookup)

	paths, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Len(t, paths, 1, "duplicate repo_id should collapse to one clone")
	require.Len(t, git.cloneCalls, 1)
	require.Equal(t, mgr.RepositoryPath("sk_shared"), paths[0])
}

func TestSyncAgent_SkipsSkillWithEmptyRepoID(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_x"}
	access := []SkillAccess{
		{SkillID: "skill_1", Name: "A", Branch: "main",
			CloneURL: "https://x/a.sk.git", RepoID: ""}, // skipped
		{SkillID: "skill_2", Name: "B", Branch: "main",
			CloneURL: "https://x/b.sk.git", RepoID: "sk_b"},
	}
	sync, _ := newTestSync(t, git, boundProvider(access).Lookup)

	paths, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Len(t, paths, 1)
	require.Len(t, git.cloneCalls, 1, "only the valid repository should be cloned")
	require.Contains(t, git.cloneCalls[0].URL, "b.sk.git")
}

func TestSyncAgent_SkipsRepositoryOnGitFailure(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_ok"}
	access := []SkillAccess{
		{SkillID: "skill_1", Name: "A", Branch: "main",
			CloneURL: "https://x/a.sk.git", RepoID: "sk_a"},
		{SkillID: "skill_2", Name: "B", Branch: "main",
			CloneURL: "https://x/b.sk.git", RepoID: "sk_b"},
	}
	// Make the first call fail so we can prove it was skipped.
	git.cloneStubFn = func(url, _, _ string) error {
		if strings.Contains(url, "a.sk.git") {
			return errors.New("network unreachable")
		}
		return nil
	}
	sync, mgr := newTestSync(t, git, boundProvider(access).Lookup)

	paths, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err, "best-effort sync never returns an error")
	require.Len(t, paths, 1, "the failed repository is dropped; the successful one survives")

	// The failed repo's record must carry Failed state so operators can see it.
	rec, ok := mgr.Get("sk_a")
	require.True(t, ok)
	require.Equal(t, profile.SkillSyncStateFailed, rec.LastSyncState)
	require.NotEmpty(t, rec.LastError)
	require.Equal(t, 1, rec.RetryCount)
}

func TestSyncAgent_EmptyAgentIDReturnsNoPaths(t *testing.T) {
	sync, _ := newTestSync(t, &fakeGit{}, boundProvider(nil).Lookup)
	paths, state, err := sync.SyncAgent(context.Background(), "   ", "")
	require.NoError(t, err)
	require.Empty(t, paths)
	require.Equal(t, dispatcher.SkillLookupUnknown, state, "degenerate input must preserve cache (Unknown)")
}

// TestSyncAgent_EmptyAccessReturnsUnbound confirms that when the provider
// returns LookupUnbound (authoritative empty), SyncAgent caches an
// authoritative-empty entry and returns nil paths.
func TestSyncAgent_EmptyAccessReturnsUnbound(t *testing.T) {
	sync, _ := newTestSync(t, &fakeGit{}, stubProvider{state: dispatcher.SkillLookupUnbound}.Lookup)
	const scope = "agt_x@ch#https://s:ws"

	paths, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupUnbound, state)
	require.Nil(t, paths)

	cachedState, has := sync.CacheState(scope)
	require.True(t, has)
	require.Equal(t, dispatcher.SkillCacheAuthoritativeEmpty, cachedState)
	require.Nil(t, sync.CachedPaths(scope))
	require.Nil(t, sync.CachedRepoIDs(scope))
}

// TestCachedRepoIDs pins the reconciler contract: after a Bound sync
// the cache exposes the deduped RepoIDs the server reported as bound (including
// ones whose clone failed — those are still bound, not orphans), and Unbound
// clears them. The daemon reconciler diffs this union against local repos.
func TestCachedRepoIDs(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc", cloneErr: nil}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "skill-a"), 0o755)
	}
	access := []SkillAccess{
		{SkillID: "s1", Name: "A", Branch: "main", CloneURL: "https://x/a.sk.git", RepoID: "sk_a"},
		{SkillID: "s2", Name: "B", Branch: "main", CloneURL: "https://x/b.sk.git", RepoID: "sk_b"},
		// Duplicate RepoID across two skills bound to the same repo — must dedupe.
		{SkillID: "s3", Name: "C", Branch: "main", CloneURL: "https://x/a.sk.git", RepoID: "sk_a"},
	}
	const scope = "agt_x@ch#https://s:ws"
	sync, _ := newTestSync(t, git, boundProvider(access).Lookup)

	_, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, state)

	repoIDs := sync.CachedRepoIDs(scope)
	require.ElementsMatch(t, []string{"sk_a", "sk_b"}, repoIDs)

	// A later authoritative Unbound must clear the cached RepoIDs so the
	// reconciler can treat this scope's former repos as candidates for removal.
	unboundSync, _ := newTestSync(t, &fakeGit{}, stubProvider{state: dispatcher.SkillLookupUnbound}.Lookup)
	// Use a fresh scope to avoid cross-contamination with the bound cache above.
	const unboundScope = "agt_y@ch#https://s:ws"
	_, _, err = unboundSync.SyncAgent(context.Background(), "agt_y", unboundScope)
	require.NoError(t, err)
	require.Nil(t, unboundSync.CachedRepoIDs(unboundScope))
}

// TestSyncAgent_ProviderErrorPreservesCache pins the three-state contract:
// a provider error is Unknown. The cache MUST survive so a transient token
// refresh failure does not wipe a prior good snapshot.
func TestSyncAgent_ProviderErrorPreservesCache(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	const scope = "agt_x@ch#https://s:ws"

	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	paths, _, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Len(t, paths, 1)
	require.Len(t, sync.CachedPaths(scope), 1, "precondition: cache populated")

	// Swap provider to fail. Cache MUST survive.
	sync.provider = stubProvider{err: errors.New("provider offline")}.Lookup
	cached, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.Error(t, err)
	require.Equal(t, dispatcher.SkillLookupUnknown, state)
	require.Equal(t, paths, cached,
		"Unknown must return prior cached paths so callers can fall back")
	require.Len(t, sync.CachedPaths(scope), 1,
		"Unknown MUST preserve cache so transient failures do not wipe good snapshots")
}

// TestSyncAgent_NilProviderPreservesCache pins the nil-provider path:
// surface Unknown with an error, do NOT touch the cache.
func TestSyncAgent_NilProviderPreservesCache(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	const scope = "agt_x@ch#https://s:ws"
	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	_, _, _ = sync.SyncAgent(context.Background(), "agt_x", scope)
	require.Len(t, sync.CachedPaths(scope), 1, "precondition: cache populated")

	sync.provider = nil
	_, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.Error(t, err)
	require.Equal(t, dispatcher.SkillLookupUnknown, state)
	require.Len(t, sync.CachedPaths(scope), 1,
		"nil provider must not wipe a prior good snapshot")
}

// TestSyncAgent_ManagedRepositoryDirIsSeparateFromManualSkillDir pins the
// hard constraint that managed skill repositories live under managed-skills/ and
// never collide with the user's manual ~/.agentteam/skills imports.
func TestSyncAgent_ManagedRepositoryDirIsSeparateFromManualSkillDir(t *testing.T) {
	git := &fakeGit{cloneRev: "rev"}
	root := t.TempDir()
	mgr := profile.New(root).ManagedSkillRepositories()
	manualDir := profile.New(root).Skills().Dir()

	sync := NewSync(boundProvider([]SkillAccess{{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}}).Lookup, git, mgr)

	paths, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Len(t, paths, 1)

	managedClone := paths[0]
	require.True(t, strings.HasPrefix(managedClone, mgr.Dir()),
		"clone path %q must live under managed dir %q", managedClone, mgr.Dir())
	require.False(t, strings.HasPrefix(managedClone, manualDir),
		"clone path %q must NOT live under manual skill dir %q", managedClone, manualDir)
	require.Equal(t, filepath.Join(root, "managed-skills"), mgr.Dir())
	require.Equal(t, filepath.Join(root, "skills"), manualDir)
	require.NotEqual(t, mgr.Dir(), manualDir)
}

func TestCountSkills_CountsSupportedSkillLayouts(t *testing.T) {
	dir := t.TempDir()
	// Root single-skill layout.
	require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# root"), 0o644))
	// Collection layout.
	require.NoError(t, os.MkdirAll(filepath.Join(dir, "a"), 0o755))
	require.NoError(t, os.WriteFile(filepath.Join(dir, "a", "SKILL.md"), []byte("# a"), 0o644))
	// Default Create template layout.
	require.NoError(t, os.MkdirAll(filepath.Join(dir, "skills", "b"), 0o755))
	require.NoError(t, os.WriteFile(filepath.Join(dir, "skills", "b", "SKILL.md"), []byte("# b"), 0o644))
	// Empty subdir (not a skill).
	require.NoError(t, os.MkdirAll(filepath.Join(dir, "c"), 0o755))
	// Deep arbitrary nesting is not a supported layout.
	require.NoError(t, os.MkdirAll(filepath.Join(dir, "deep", "nested"), 0o755))
	require.NoError(t, os.WriteFile(filepath.Join(dir, "deep", "nested", "SKILL.md"), []byte("# deep"), 0o644))
	// Regular file at top level.
	require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("# readme"), 0o644))

	require.Equal(t, 3, countSkills(dir))
}

func TestCountSkills_ReturnsZeroOnMissingDir(t *testing.T) {
	require.Equal(t, 0, countSkills(filepath.Join(t.TempDir(), "nope")))
}

// TestSyncAgent_ThreadsAuthThroughToGitRunner proves the AGS sync lease
// auth method from the provider reaches the production GitRunner. The sync
// producer must not silently drop auth on the floor — that would cause every
// HTTPS clone/fetch to fail with a 401.
func TestSyncAgent_ThreadsAuthThroughToGitRunner(t *testing.T) {
	git := &fakeGit{cloneRev: "rev"}
	wantAuth := &gittransport.TokenAuth{Token: "ags_rat_test"}
	sk := SkillAccess{
		SkillID:  "skill_1",
		Name:     "Backend",
		Branch:   "main",
		CloneURL: "https://agentteam.example.com/agentteam/ws-1.sk.git",
		RepoID:   "sk_abc",
		Auth:     wantAuth,
	}
	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)

	_, _, err := sync.SyncAgent(context.Background(), "agt_xxx", "agt_xxx@ch_test#https://test:ws_test")
	require.NoError(t, err)
	require.Len(t, git.cloneCalls, 1)
	require.Equal(t, wantAuth, git.cloneCalls[0].Auth,
		"Clone must receive the auth method from SkillAccess")
}

// ---------------------------------------------------------------------------
// Cache semantics (per-scope snapshot used by execution-launch paths)
// ---------------------------------------------------------------------------

// TestSyncAgent_CachesSuccessfulResult proves the most recent successful
// sync is retrievable via CachedPaths. Execution sub-agents use this
// to avoid re-running the network sync per attempt.
func TestSyncAgent_CachesSuccessfulResult(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	const scope = "agt_x@ch#https://s:ws"

	paths, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupBound, state)
	require.Len(t, paths, 1)

	cached := sync.CachedPaths(scope)
	require.Equal(t, paths, cached, "CachedPaths must return last successful sync result")

	cachedState, has := sync.CacheState(scope)
	require.True(t, has)
	require.Equal(t, dispatcher.SkillCachePopulated, cachedState)
}

// TestSyncAgent_EmptyResultOverwritesCache pins the unbind/revocation
// path: when a previously-bound agent returns LookupUnbound, the cache for
// that scope MUST be overwritten to AuthoritativeEmpty so CachedPaths
// stops surfacing the stale paths.
func TestSyncAgent_EmptyResultOverwritesCache(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	const scope = "agt_x@ch#https://s:ws"

	// First sync returns the binding; cache should be non-empty.
	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	paths, _, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Len(t, paths, 1)
	require.Len(t, sync.CachedPaths(scope), 1)

	// Swap provider to simulate unbind/revocation → Unbound. The Sync must
	// overwrite the cache so execution paths stop seeing the old path.
	sync.provider = stubProvider{state: dispatcher.SkillLookupUnbound}.Lookup
	_, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Equal(t, dispatcher.SkillLookupUnbound, state)
	require.Nil(t, sync.CachedPaths(scope),
		"Unbound must overwrite cache so stale paths are not exposed")

	cachedState, has := sync.CacheState(scope)
	require.True(t, has)
	require.Equal(t, dispatcher.SkillCacheAuthoritativeEmpty, cachedState)
}

// TestSyncAgent_UnknownPreservesCache pins the three-state invariant: a
// provider error or Unknown state MUST NOT touch the cache. A prior good
// snapshot survives so transient failures (token refresh flake, 5xx) do not
// wipe bindings.
func TestSyncAgent_UnknownPreservesCache(t *testing.T) {
	git := &fakeGit{cloneRev: "rev_abc"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	const scope = "agt_x@ch#https://s:ws"

	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)
	paths, _, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.NoError(t, err)
	require.Len(t, paths, 1)
	require.Len(t, sync.CachedPaths(scope), 1, "precondition: cache populated")
	populatedState, _ := sync.CacheState(scope)
	require.Equal(t, dispatcher.SkillCachePopulated, populatedState)

	// Provider fails — cache MUST survive.
	sync.provider = stubProvider{err: errors.New("provider offline")}.Lookup
	cached, state, err := sync.SyncAgent(context.Background(), "agt_x", scope)
	require.Error(t, err)
	require.Equal(t, dispatcher.SkillLookupUnknown, state)
	require.Equal(t, paths, cached, "Unknown returns the prior cached paths so callers can fall back")
	require.Len(t, sync.CachedPaths(scope), 1,
		"Unknown MUST preserve cache so transient failures do not wipe good snapshots")
	stillPopulated, _ := sync.CacheState(scope)
	require.Equal(t, dispatcher.SkillCachePopulated, stillPopulated,
		"cache state must not regress to Miss on Unknown")
}

// TestSyncAgent_CacheKeyedByScope proves the cache does not collide
// across scopes. Two distinct scopeKeys for the same agentID must maintain
// independent cache entries — this is what prevents same-agent-different-
// workspace leaks.
func TestSyncAgent_CacheKeyedByScope(t *testing.T) {
	git := &fakeGit{cloneRev: "rev"}
	git.cloneStubFn = func(_, _, dest string) error {
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	sync, _ := newTestSync(t, git, boundProvider([]SkillAccess{sk}).Lookup)

	const scopeA = "agt_x@ch_A#https://sA:ws_A"
	const scopeB = "agt_x@ch_B#https://sB:ws_B"

	pathsA, _, err := sync.SyncAgent(context.Background(), "agt_x", scopeA)
	require.NoError(t, err)
	require.Len(t, pathsA, 1)

	// Scope B has never been synced — must miss.
	require.Nil(t, sync.CachedPaths(scopeB),
		"unsynced scope must miss cache")
	missState, has := sync.CacheState(scopeB)
	require.False(t, has, "unsynced scope must report CacheMiss with has=false")
	require.Equal(t, dispatcher.SkillCacheMiss, missState)

	// Scope A's cache survives scope B's existence check.
	require.Equal(t, pathsA, sync.CachedPaths(scopeA),
		"syncing scope A must not pollute scope B's cache or vice versa")
}

// TestCachedPaths_EmptyScopeReturnsNil pins the degenerate case.
func TestCachedPaths_EmptyScopeReturnsNil(t *testing.T) {
	sync, _ := newTestSync(t, &fakeGit{}, boundProvider(nil).Lookup)
	require.Nil(t, sync.CachedPaths(""))
	require.Nil(t, sync.CachedPaths("   "))
}

// TestSyncAgent_SingleflightDedupesConcurrentRepoSyncs pins the
// singleflight guarantee: two concurrent SyncAgent calls targeting the
// same RepoID share a single clone/fetch — they never race to write the same
// directory. This is the F6 mitigation.
func TestSyncAgent_SingleflightDedupesConcurrentRepoSyncs(t *testing.T) {
	git := &fakeGit{cloneRev: "rev"}
	var (
		mu              stdlibsync.Mutex
		cloneCnt        int
		lookupCnt       atomic.Int32
		cloneStarted    = make(chan struct{})
		allLookupsReady = make(chan struct{})
		releaseClone    = make(chan struct{})
		closeCloneOnce  stdlibsync.Once
		closeLookupOnce stdlibsync.Once
	)
	git.cloneStubFn = func(_, _, dest string) error {
		mu.Lock()
		cloneCnt++
		mu.Unlock()
		closeCloneOnce.Do(func() { close(cloneStarted) })
		<-releaseClone
		return os.MkdirAll(filepath.Join(dest, "sk-a"), 0o755)
	}
	sk := SkillAccess{
		SkillID: "skill_1", Name: "A", Branch: "main",
		CloneURL: "https://x/a.sk.git", RepoID: "sk_a",
	}
	const N = 5
	provider := func(context.Context, string) ([]SkillAccess, dispatcher.SkillLookupState, error) {
		if lookupCnt.Add(1) == N {
			closeLookupOnce.Do(func() { close(allLookupsReady) })
		}
		return []SkillAccess{sk}, dispatcher.SkillLookupBound, nil
	}
	sync, _ := newTestSync(t, git, provider)
	const scope = "agt_x@ch#https://s:ws"

	var wg stdlibsync.WaitGroup
	start := make(chan struct{})
	wg.Add(N)
	for i := 0; i < N; i++ {
		go func() {
			defer wg.Done()
			<-start
			_, _, _ = sync.SyncAgent(context.Background(), "agt_x", scope)
		}()
	}
	close(start)
	<-cloneStarted
	<-allLookupsReady
	close(releaseClone)
	wg.Wait()

	require.Equal(t, 1, cloneCnt, "singleflight must collapse N concurrent syncs of the same repo to one clone")
}