diff --git a/internal/fetchsvc/ratelimit.go b/internal/fetchsvc/ratelimit.go new file mode 100644 index 0000000000..d3ef40af9c --- /dev/null +++ b/internal/fetchsvc/ratelimit.go @@ -0,0 +1,60 @@ +package fetchsvc + +import ( + "math" + "sync/atomic" +) + +const DefaultMaxFetches = 10 + +// RateLimiter enforces a maximum number of runtime fetches per agent run. +// It uses an atomic counter for thread safety without requiring a mutex. +type RateLimiter struct { + max int32 + current atomic.Int32 +} + +// NewRateLimiter creates a rate limiter with the given maximum. +// If max <= 0, DefaultMaxFetches is used. +func NewRateLimiter(max int) *RateLimiter { + if max <= 0 { + max = DefaultMaxFetches + } + if max > math.MaxInt32 { + max = math.MaxInt32 + } + return &RateLimiter{max: int32(max)} +} + +// Allow checks if another fetch is permitted. Returns true and increments +// the counter atomically if under the limit. Thread-safe for concurrent use. +func (r *RateLimiter) Allow() bool { + for { + cur := r.current.Load() + if cur >= r.max { + return false + } + if r.current.CompareAndSwap(cur, cur+1) { + return true + } + } +} + +// Release returns a previously consumed slot to the pool. +// It is safe to call even if no slot was consumed; the counter will not go below zero. +func (r *RateLimiter) Release() { + for { + cur := r.current.Load() + if cur <= 0 { + return + } + if r.current.CompareAndSwap(cur, cur-1) { + return + } + } +} + +// Count returns the current number of fetches performed. +func (r *RateLimiter) Count() int32 { + return r.current.Load() +} diff --git a/internal/fetchsvc/ratelimit_test.go b/internal/fetchsvc/ratelimit_test.go new file mode 100644 index 0000000000..b603274fa5 --- /dev/null +++ b/internal/fetchsvc/ratelimit_test.go @@ -0,0 +1,130 @@ +package fetchsvc + +import ( + "sync" + "sync/atomic" + "testing" +) + +func TestRateLimiter_AllowsUpToMax(t *testing.T) { + r := NewRateLimiter(3) + + for i := 0; i < 3; i++ { + if !r.Allow() { + t.Fatalf("Allow() returned false on call %d, want true", i+1) + } + } + if r.Allow() { + t.Fatal("Allow() returned true after max reached, want false") + } +} + +func TestRateLimiter_RejectsAboveMax(t *testing.T) { + r := NewRateLimiter(1) + + if !r.Allow() { + t.Fatal("first Allow() returned false") + } + for i := 0; i < 5; i++ { + if r.Allow() { + t.Fatalf("Allow() returned true on extra call %d", i+1) + } + } +} + +func TestRateLimiter_DefaultMax(t *testing.T) { + r := NewRateLimiter(0) + if r.max != DefaultMaxFetches { + t.Fatalf("max = %d, want %d", r.max, DefaultMaxFetches) + } + + r2 := NewRateLimiter(-5) + if r2.max != DefaultMaxFetches { + t.Fatalf("max = %d, want %d", r2.max, DefaultMaxFetches) + } +} + +func TestRateLimiter_Count(t *testing.T) { + r := NewRateLimiter(5) + + if got := r.Count(); got != 0 { + t.Fatalf("Count() = %d before any calls, want 0", got) + } + + r.Allow() + r.Allow() + if got := r.Count(); got != 2 { + t.Fatalf("Count() = %d after 2 Allow(), want 2", got) + } +} + +func TestRateLimiter_ConcurrentSafety(t *testing.T) { + const max = 50 + const goroutines = 200 + + r := NewRateLimiter(max) + + var allowed atomic.Int32 + var wg sync.WaitGroup + wg.Add(goroutines) + + for i := 0; i < goroutines; i++ { + go func() { + defer wg.Done() + if r.Allow() { + allowed.Add(1) + } + }() + } + + wg.Wait() + + if got := allowed.Load(); got != max { + t.Fatalf("total allowed = %d across %d goroutines, want exactly %d", got, goroutines, max) + } + if got := r.Count(); got != int32(max) { + t.Fatalf("Count() = %d, want %d", got, max) + } +} + +func TestRateLimiter_Release(t *testing.T) { + r := NewRateLimiter(1) + + if !r.Allow() { + t.Fatal("first Allow() should succeed") + } + if r.Allow() { + t.Fatal("second Allow() should fail (at max)") + } + + r.Release() + + if got := r.Count(); got != 0 { + t.Fatalf("Count() = %d after Release(), want 0", got) + } + if !r.Allow() { + t.Fatal("Allow() should succeed after Release()") + } +} + +func TestRateLimiter_ReleaseFloor(t *testing.T) { + r := NewRateLimiter(3) + + // Release without prior Allow should not go negative. + r.Release() + r.Release() + + if got := r.Count(); got != 0 { + t.Fatalf("Count() = %d after Release() with no Allow(), want 0", got) + } + + // Should still allow up to max. + for i := 0; i < 3; i++ { + if !r.Allow() { + t.Fatalf("Allow() returned false on call %d after floor-guarded Release()", i+1) + } + } + if r.Allow() { + t.Fatal("Allow() should fail after reaching max") + } +} diff --git a/internal/fetchsvc/service.go b/internal/fetchsvc/service.go new file mode 100644 index 0000000000..b06adbf68d --- /dev/null +++ b/internal/fetchsvc/service.go @@ -0,0 +1,275 @@ +// Package fetchsvc provides a runtime skill fetch service for agents running +// in sandboxes. It validates, fetches, caches, and uploads skill directories +// on behalf of in-sandbox agent processes. +package fetchsvc + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "path/filepath" + "time" + + "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/harness" +) + +const maxRequestBytes = 1 << 20 // 1 MB + +// fetchError carries an HTTP status code alongside the error message, +// eliminating string-based error classification for status mapping. +type fetchError struct { + msg string + status int +} + +func (e *fetchError) Error() string { return e.msg } + +// FetchRequest is a runtime skill fetch request from an in-sandbox agent. +type FetchRequest struct { + URL string `json:"url"` // full URL including #sha256= +} + +// FetchResponse is returned after a fetch attempt. +type FetchResponse struct { + LocalPath string `json:"local_path,omitempty"` // sandbox-local skill directory path + Error string `json:"error,omitempty"` +} + +// Uploader abstracts uploading skill directories into the sandbox. +type Uploader interface { + UploadSkillDir(sandboxName, localPath, remotePath string) error +} + +// ServiceConfig holds configuration for creating a new Service. +type ServiceConfig struct { + Harness *harness.Harness + ForgeClient forge.Client + FetchPolicy fetch.FetchPolicy + WorkspaceRoot string // host-side root for .fullsend-cache/ + AuditLogPath string + TraceID string + SandboxName string + MaxFetches int // 0 → DefaultMaxFetches (10) + Uploader Uploader // nil → skip upload step + SkillDestDir string // "" → /sandbox/claude-config/skills +} + +// Service handles runtime skill fetch requests from agents running in sandboxes. +type Service struct { + harness *harness.Harness + forgeClient forge.Client + fetchPolicy fetch.FetchPolicy + workspaceRoot string + auditLogPath string + traceID string + sandboxName string + uploader Uploader + skillDestDir string + limiter *RateLimiter +} + +// New creates a runtime fetch service. +func New(cfg ServiceConfig) *Service { + skillDest := cfg.SkillDestDir + if skillDest == "" { + skillDest = "/sandbox/claude-config/skills" + } + return &Service{ + harness: cfg.Harness, + forgeClient: cfg.ForgeClient, + fetchPolicy: cfg.FetchPolicy, + workspaceRoot: cfg.WorkspaceRoot, + auditLogPath: cfg.AuditLogPath, + traceID: cfg.TraceID, + sandboxName: cfg.SandboxName, + uploader: cfg.Uploader, + skillDestDir: skillDest, + limiter: NewRateLimiter(cfg.MaxFetches), + } +} + +// HandleFetch processes a single runtime skill fetch request. +// On success it returns a FetchResponse with LocalPath set and a nil error. +// On failure it returns a *fetchError with an appropriate HTTP status code. +func (s *Service) HandleFetch(ctx context.Context, req FetchRequest) (FetchResponse, error) { + if req.URL == "" { + return FetchResponse{}, &fetchError{"url is required", http.StatusBadRequest} + } + + if !harness.IsURL(req.URL) { + return FetchResponse{}, &fetchError{"url must be a valid HTTPS URL", http.StatusBadRequest} + } + + cleanURL, expectedHash, hasHash := harness.ParseIntegrityHash(req.URL) + if !hasHash { + return FetchResponse{}, &fetchError{"url must include #sha256=... integrity hash", http.StatusBadRequest} + } + + allowedBy := s.harness.MatchingAllowedPrefix(cleanURL) + if allowedBy == "" { + return FetchResponse{}, &fetchError{ + fmt.Sprintf("url %q is not in allowed_remote_resources", cleanURL), + http.StatusForbidden, + } + } + + forgeInfo, err := forge.ParseForgeURL(cleanURL) + if err != nil { + return FetchResponse{}, &fetchError{"skill URLs must be hosted on a supported forge", http.StatusBadRequest} + } + + if forgeInfo.Path == "" { + return FetchResponse{}, &fetchError{"skill URL must include a path to a directory", http.StatusBadRequest} + } + + if !s.limiter.Allow() { + return FetchResponse{}, &fetchError{ + "runtime fetch rate limit exceeded", + http.StatusTooManyRequests, + } + } + committed := false + defer func() { + if !committed { + s.limiter.Release() + } + }() + + treePath, dirEntry, err := fetch.CacheGetDir(s.workspaceRoot, expectedHash) + if err != nil { + return FetchResponse{}, &fetchError{"internal error during cache lookup", http.StatusInternalServerError} + } + + cacheHit := treePath != "" + fetchedAt := time.Now().UTC() + + if !cacheHit { + if s.forgeClient == nil { + return FetchResponse{}, &fetchError{"forge client is required to fetch uncached skill", http.StatusInternalServerError} + } + if s.fetchPolicy.Offline { + return FetchResponse{}, &fetchError{"skill not in cache and offline mode is enabled", http.StatusServiceUnavailable} + } + + entries, err := s.forgeClient.ListDirectoryContents(ctx, forgeInfo.Owner, forgeInfo.Repo, forgeInfo.Path, forgeInfo.Ref, true) + if err != nil { + return FetchResponse{}, &fetchError{"failed to list skill directory from forge", http.StatusBadGateway} + } + + files := make(map[string][]byte) + for _, e := range entries { + if e.Type != "file" { + continue + } + fullPath := forgeInfo.Path + "/" + e.Path + content, err := s.forgeClient.GetFileContentAtRef(ctx, forgeInfo.Owner, forgeInfo.Repo, fullPath, forgeInfo.Ref) + if err != nil { + return FetchResponse{}, &fetchError{"failed to fetch skill file from forge", http.StatusBadGateway} + } + files[e.Path] = content + } + + if len(files) == 0 { + return FetchResponse{}, &fetchError{"skill directory contains no files", http.StatusUnprocessableEntity} + } + + actualHash := fetch.ComputeTreeHash(files) + if actualHash != expectedHash { + return FetchResponse{}, &fetchError{ + "integrity check failed", + http.StatusUnprocessableEntity, + } + } + + if _, err := fetch.CachePutDir(s.workspaceRoot, cleanURL, files); err != nil { + return FetchResponse{}, &fetchError{"failed to cache skill directory", http.StatusInternalServerError} + } + + cachePath, err := fetch.CachePath(s.workspaceRoot, expectedHash) + if err != nil { + return FetchResponse{}, &fetchError{"internal error computing cache path", http.StatusInternalServerError} + } + treePath = filepath.Join(cachePath, "tree") + } else if dirEntry != nil { + fetchedAt = dirEntry.FetchTime + } + + basename := filepath.Base(forgeInfo.Path) + if basename == "" || basename == "." { + basename = "skill" + } + hashPrefix := expectedHash + if len(hashPrefix) > 8 { + hashPrefix = hashPrefix[:8] + } + remotePath := filepath.Join(s.skillDestDir, hashPrefix+"-"+basename) + + if s.uploader != nil { + if err := s.uploader.UploadSkillDir(s.sandboxName, treePath, remotePath); err != nil { + return FetchResponse{}, &fetchError{"failed to upload skill to sandbox", http.StatusInternalServerError} + } + } + + if s.auditLogPath != "" { + _ = fetch.AppendFetchAudit(s.auditLogPath, fetch.FetchAuditEntry{ + TraceID: s.traceID, + FetchTime: fetchedAt, + URL: cleanURL, + SHA256: expectedHash, + FetchType: "runtime", + AllowedBy: allowedBy, + CacheHit: cacheHit, + }) + } + + committed = true + return FetchResponse{LocalPath: remotePath}, nil +} + +// ServeHTTP implements http.Handler for HTTP-based transports. +func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxRequestBytes) + + var req FetchRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeJSON(w, http.StatusBadRequest, FetchResponse{Error: "invalid request body"}) + return + } + + resp, err := s.HandleFetch(r.Context(), req) + if err != nil { + var fe *fetchError + status := http.StatusInternalServerError + if errors.As(err, &fe) { + status = fe.status + resp.Error = fe.msg + } else { + resp.Error = "internal error" + } + writeJSON(w, status, resp) + return + } + + writeJSON(w, http.StatusOK, resp) +} + +func writeJSON(w http.ResponseWriter, status int, v any) { + data, err := json.Marshal(v) + if err != nil { + http.Error(w, `{"error":"response encoding failed"}`, http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write(data) + _, _ = w.Write([]byte("\n")) +} diff --git a/internal/fetchsvc/service_test.go b/internal/fetchsvc/service_test.go new file mode 100644 index 0000000000..028d098d81 --- /dev/null +++ b/internal/fetchsvc/service_test.go @@ -0,0 +1,537 @@ +package fetchsvc + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/fullsend-ai/fullsend/internal/fetch" + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/harness" +) + +// stubUploader records upload calls without requiring openshell. +type stubUploader struct { + calls []uploadCall +} + +type uploadCall struct { + sandboxName, localPath, remotePath string +} + +func (u *stubUploader) UploadSkillDir(sandboxName, localPath, remotePath string) error { + u.calls = append(u.calls, uploadCall{sandboxName, localPath, remotePath}) + return nil +} + +// testHarness returns a Harness with allowed_remote_resources set. +func testHarness(prefixes ...string) *harness.Harness { + return &harness.Harness{ + Agent: "agents/code.md", + AllowedRemoteResources: prefixes, + } +} + +// fakeSkillFiles returns a minimal skill directory for testing. +func fakeSkillFiles() map[string][]byte { + return map[string][]byte{ + "SKILL.md": []byte("---\nname: test-skill\n---\n# Test Skill\n"), + } +} + +// fakeSkillHash returns the tree hash for fakeSkillFiles(). +func fakeSkillHash() string { + return fetch.ComputeTreeHash(fakeSkillFiles()) +} + +// setupFakeForge configures a FakeClient with a skill directory at the given owner/repo/path@ref. +func setupFakeForge(owner, repo, dirPath, ref string, files map[string][]byte) *forge.FakeClient { + fc := forge.NewFakeClient() + var entries []forge.DirectoryEntry + for p := range files { + entries = append(entries, forge.DirectoryEntry{Path: p, Type: "file", Size: len(files[p])}) + } + fc.DirContents[owner+"/"+repo+"/"+dirPath+"@"+ref] = entries + for p, content := range files { + fc.FileContentsRef[owner+"/"+repo+"/"+dirPath+"/"+p+"@"+ref] = content + } + return fc +} + +func TestHandleFetch_CacheHit(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + + // Pre-populate cache. + fetch.CachePutDir(tmpDir, "https://github.com/org/repo/tree/abc123/skills/test-skill", files) + + uploader := &stubUploader{} + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: tmpDir, + MaxFetches: 10, + Uploader: uploader, + SkillDestDir: "/sandbox/skills", + AuditLogPath: filepath.Join(tmpDir, "audit.jsonl"), + TraceID: "trace-1", + }) + + resp, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + hash, + }) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(resp.LocalPath, "test-skill") { + t.Fatalf("LocalPath %q should contain skill name", resp.LocalPath) + } + if len(uploader.calls) != 1 { + t.Fatalf("expected 1 upload call, got %d", len(uploader.calls)) + } + + // Verify audit log was written. + auditData, err := os.ReadFile(filepath.Join(tmpDir, "audit.jsonl")) + if err != nil { + t.Fatalf("reading audit log: %v", err) + } + if !strings.Contains(string(auditData), `"fetch_type":"runtime"`) { + t.Fatal("audit log should contain fetch_type runtime") + } + if !strings.Contains(string(auditData), `"cache_hit":true`) { + t.Fatal("audit log should show cache_hit true") + } +} + +func TestHandleFetch_ForgeFetch(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + + fc := setupFakeForge("org", "repo", "skills/test-skill", "abc123", files) + uploader := &stubUploader{} + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + ForgeClient: fc, + WorkspaceRoot: tmpDir, + MaxFetches: 10, + Uploader: uploader, + SkillDestDir: "/sandbox/skills", + AuditLogPath: filepath.Join(tmpDir, "audit.jsonl"), + TraceID: "trace-2", + }) + + resp, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + hash, + }) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.LocalPath == "" { + t.Fatal("expected non-empty LocalPath") + } + + // Verify cache was populated. + treePath, _, cacheErr := fetch.CacheGetDir(tmpDir, hash) + if cacheErr != nil { + t.Fatalf("cache lookup: %v", cacheErr) + } + if treePath == "" { + t.Fatal("skill should be cached after fetch") + } + + // Verify audit log shows cache miss. + auditData, _ := os.ReadFile(filepath.Join(tmpDir, "audit.jsonl")) + if !strings.Contains(string(auditData), `"cache_hit":false`) { + t.Fatal("audit log should show cache_hit false for fresh fetch") + } +} + +func TestHandleFetch_NotInAllowlist(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/allowed-org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/other-org/repo/tree/abc123/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + + if err == nil { + t.Fatal("expected error for URL not in allowlist") + } + if !strings.Contains(err.Error(), "not in allowed_remote_resources") { + t.Fatalf("error should mention allowlist: %v", err) + } +} + +func TestHandleFetch_MissingHash(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/foo", + }) + + if err == nil { + t.Fatal("expected error for missing hash") + } + if !strings.Contains(err.Error(), "integrity hash") { + t.Fatalf("error should mention integrity hash: %v", err) + } +} + +func TestHandleFetch_IntegrityMismatch(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + wrongHash := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + fc := setupFakeForge("org", "repo", "skills/test-skill", "abc123", files) + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + ForgeClient: fc, + WorkspaceRoot: tmpDir, + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + wrongHash, + }) + + if err == nil { + t.Fatal("expected integrity error") + } + if !strings.Contains(err.Error(), "integrity check failed") { + t.Fatalf("error should mention integrity check: %v", err) + } +} + +func TestHandleFetch_RateLimitExceeded(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + + // Pre-populate cache so requests succeed until rate limit hits. + fetch.CachePutDir(tmpDir, "https://github.com/org/repo/tree/abc123/skills/test-skill", files) + + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: tmpDir, + MaxFetches: 2, + SkillDestDir: "/sandbox/skills", + }) + + url := "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + hash + + // First 2 should succeed. + for i := 0; i < 2; i++ { + _, err := svc.HandleFetch(context.Background(), FetchRequest{URL: url}) + if err != nil { + t.Fatalf("request %d failed: %v", i+1, err) + } + } + + // 3rd should fail. + _, err := svc.HandleFetch(context.Background(), FetchRequest{URL: url}) + if err == nil { + t.Fatal("expected rate limit error") + } + if !strings.Contains(err.Error(), "rate limit exceeded") { + t.Fatalf("error should mention rate limit: %v", err) + } +} + +func TestHandleFetch_RateLimitRollbackOnFailure(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 1, + }) + + // First request consumes a slot but fails (no forge client, cache miss). + // The slot should be rolled back. + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + if err == nil { + t.Fatal("expected error for missing forge client") + } + + // Second request should NOT be rate-limited because the first slot was released. + _, err = svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/bar#sha256=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + }) + if err == nil { + t.Fatal("expected error (no forge client)") + } + if strings.Contains(err.Error(), "rate limit exceeded") { + t.Fatal("should not be rate-limited; failed slots should be rolled back") + } +} + +func TestHandleFetch_NonForgeURL(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://example.com/skills/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://example.com/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + + if err == nil { + t.Fatal("expected error for non-forge URL") + } + if !strings.Contains(err.Error(), "supported forge") { + t.Fatalf("error should mention forge: %v", err) + } +} + +func TestHandleFetch_OfflineMode(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + FetchPolicy: fetch.FetchPolicy{ + Offline: true, + }, + ForgeClient: forge.NewFakeClient(), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + + if err == nil { + t.Fatal("expected error in offline mode with cache miss") + } + if !strings.Contains(err.Error(), "offline") { + t.Fatalf("error should mention offline: %v", err) + } +} + +func TestHandleFetch_EmptyURL(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{URL: ""}) + if err == nil { + t.Fatal("expected error for empty URL") + } +} + +func TestHandleFetch_InvalidURL(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{URL: "not-a-url"}) + if err == nil { + t.Fatal("expected error for invalid URL") + } +} + +func TestServeHTTP_Success(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + + fetch.CachePutDir(tmpDir, "https://github.com/org/repo/tree/abc123/skills/test-skill", files) + + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: tmpDir, + MaxFetches: 10, + SkillDestDir: "/sandbox/skills", + }) + + body, _ := json.Marshal(FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + hash, + }) + req := httptest.NewRequest(http.MethodPost, "/fetch", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + svc.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", rec.Code, rec.Body.String()) + } + + var resp FetchResponse + json.NewDecoder(rec.Body).Decode(&resp) + if resp.Error != "" { + t.Fatalf("unexpected error in response: %s", resp.Error) + } + if resp.LocalPath == "" { + t.Fatal("expected non-empty LocalPath") + } +} + +func TestServeHTTP_MethodNotAllowed(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + req := httptest.NewRequest(http.MethodGet, "/fetch", nil) + rec := httptest.NewRecorder() + + svc.ServeHTTP(rec, req) + + if rec.Code != http.StatusMethodNotAllowed { + t.Fatalf("status = %d, want 405", rec.Code) + } +} + +func TestServeHTTP_BadJSON(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + req := httptest.NewRequest(http.MethodPost, "/fetch", strings.NewReader("not json")) + rec := httptest.NewRecorder() + + svc.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestServeHTTP_Forbidden(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/allowed-org/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + body, _ := json.Marshal(FetchRequest{ + URL: "https://github.com/other-org/repo/tree/abc123/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + req := httptest.NewRequest(http.MethodPost, "/fetch", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + svc.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403", rec.Code) + } +} + +func TestServeHTTP_RateLimit(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + fetch.CachePutDir(tmpDir, "https://github.com/org/repo/tree/abc123/skills/test-skill", files) + + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: tmpDir, + MaxFetches: 1, + SkillDestDir: "/sandbox/skills", + }) + + url := "https://github.com/org/repo/tree/abc123/skills/test-skill#sha256=" + hash + + // First request succeeds. + body, _ := json.Marshal(FetchRequest{URL: url}) + req := httptest.NewRequest(http.MethodPost, "/fetch", bytes.NewReader(body)) + rec := httptest.NewRecorder() + svc.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("first request: status = %d, want 200", rec.Code) + } + + // Second request should be rate limited. + body, _ = json.Marshal(FetchRequest{URL: url}) + req = httptest.NewRequest(http.MethodPost, "/fetch", bytes.NewReader(body)) + rec = httptest.NewRecorder() + svc.ServeHTTP(rec, req) + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("second request: status = %d, want 429", rec.Code) + } +} + +func TestHandleFetch_NoForgeClient(t *testing.T) { + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: t.TempDir(), + MaxFetches: 10, + }) + + _, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/foo#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }) + + if err == nil { + t.Fatal("expected error when forge client is nil") + } + if !strings.Contains(err.Error(), "forge client is required") { + t.Fatalf("error should mention forge client: %v", err) + } +} + +func TestHandleFetch_UploadCalled(t *testing.T) { + tmpDir := t.TempDir() + files := fakeSkillFiles() + hash := fakeSkillHash() + fetch.CachePutDir(tmpDir, "https://github.com/org/repo/tree/abc123/skills/my-skill", files) + + uploader := &stubUploader{} + svc := New(ServiceConfig{ + Harness: testHarness("https://github.com/org/repo/"), + WorkspaceRoot: tmpDir, + MaxFetches: 10, + Uploader: uploader, + SandboxName: "sandbox-1", + SkillDestDir: "/sandbox/claude-config/skills", + }) + + resp, err := svc.HandleFetch(context.Background(), FetchRequest{ + URL: "https://github.com/org/repo/tree/abc123/skills/my-skill#sha256=" + hash, + }) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(uploader.calls) != 1 { + t.Fatalf("expected 1 upload, got %d", len(uploader.calls)) + } + call := uploader.calls[0] + if call.sandboxName != "sandbox-1" { + t.Fatalf("sandboxName = %q, want sandbox-1", call.sandboxName) + } + if !strings.Contains(call.remotePath, "my-skill") { + t.Fatalf("remotePath %q should contain skill name", call.remotePath) + } + if !strings.HasPrefix(call.remotePath, "/sandbox/claude-config/skills/") { + t.Fatalf("remotePath %q should start with skill dest dir", call.remotePath) + } + // Hash prefix should be in the path. + if !strings.Contains(call.remotePath, hash[:8]) { + t.Fatalf("remotePath %q should contain hash prefix %s", call.remotePath, hash[:8]) + } + _ = resp // LocalPath verified via uploader.calls +}