Go Skills

Go Skills for LlamaFarm CLI

Shared Go best practices for LlamaFarm CLI development. These guidelines ensure idiomatic, maintainable, and secure Go code.

Tech Stack

• Go 1.24+
• Cobra (CLI framework)
• Bubbletea (TUI framework)
• Lipgloss (terminal styling)

Directory Structure

cli/
  cmd/           # Command implementations
    config/      # Configuration types and loading
    orchestrator/ # Service management
    utils/       # Shared utilities
    version/     # Version and upgrade handling
  internal/      # Internal packages
    tui/         # TUI components
    buildinfo/   # Build information

Quick Reference

Error Handling

• Always wrap errors with context: fmt.Errorf("operation failed: %w", err)
• Use sentinel errors for expected conditions: var ErrNotFound = errors.New("not found")
• Check errors immediately after function calls

Concurrency

• Use sync.Mutex for shared state protection
• Use sync.RWMutex when reads dominate writes
• Use channels for goroutine communication
• Always use defer for mutex unlocks

Testing

• Use table-driven tests for comprehensive coverage
• Use interfaces for mockability
• Test file names: *_test.go in same package

Security

• Never log credentials or tokens
• Redact sensitive headers in debug logs
• Validate all external input
• Use context.Context for cancellation

Checklist Files

Go Proverbs to Remember

1. "Don't communicate by sharing memory; share memory by communicating" 2. "Errors are values" 3. "A little copying is better than a little dependency" 4. "Clear is better than clever" 5. "Design the architecture, name the components, document the details"

Common Patterns in This Codebase

HTTP Client Interface

type HTTPClient interface {
    Do(req *http.Request) (*http.Response, error)
}

Process Management with Mutex

type ProcessManager struct {
    mu        sync.RWMutex
    processes map[string]*ProcessInfo
}

Cobra Command Pattern

var myCmd = &cobra.Command{
    Use:   "mycommand",
    Short: "Brief description",
    RunE: func(cmd *cobra.Command, args []string) error {
        // Implementation
        return nil
    },
}

Bubbletea Model Pattern

type myModel struct {
    // State fields
}

func (m myModel) Init() tea.Cmd { return nil }
func (m myModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { /* ... */ }
func (m myModel) View() string { return "" }

Concurrency Patterns

Best practices for goroutines, channels, and synchronization in the LlamaFarm CLI.

Checklist

1. Protect Shared State with Mutex

Description: All shared mutable state must be protected by a mutex.

Search Pattern:

grep -rn "sync\.Mutex\|sync\.RWMutex" cli/ --include="*.go"

Pass Criteria: Every struct with shared mutable state includes a mutex. Access is synchronized.

Fail Criteria: Shared state accessed without synchronization. Race conditions possible.

Severity: Critical

Recommendation:

type ProcessManager struct {
    mu        sync.RWMutex          // Protects processes map
    processes map[string]*ProcessInfo
}

func (pm *ProcessManager) GetProcess(name string) (*ProcessInfo, bool) {
    pm.mu.RLock()
    defer pm.mu.RUnlock()
    proc, ok := pm.processes[name]
    return proc, ok
}

---

2. Use RWMutex When Reads Dominate

Description: Use sync.RWMutex when read operations significantly outnumber writes.

Search Pattern:

grep -rn "RLock\|RUnlock" cli/ --include="*.go"

Pass Criteria: Read-heavy operations use RLock/RUnlock. Write operations use Lock/Unlock.

Fail Criteria: Using sync.Mutex for read-heavy workloads, causing unnecessary contention.

Severity: Medium

Recommendation:

func (pm *ProcessManager) GetProcessStatus(name string) (string, error) {
    pm.mu.RLock()  // Multiple readers can proceed
    defer pm.mu.RUnlock()

    proc, found := pm.findProcess(name)
    if !found {
        return "", fmt.Errorf("process %s not found", name)
    }
    return proc.Status, nil
}

---

3. Always Defer Mutex Unlock

Description: Use defer for mutex unlock to ensure release on all code paths.

Search Pattern:

grep -rn "\.Lock()" -A2 cli/ --include="*.go" | grep -v "defer"

Pass Criteria: Every Lock() is immediately followed by defer Unlock().

Fail Criteria: Manual unlock at multiple return points, risk of deadlock on panic.

Severity: Critical

Recommendation:

func (pm *ProcessManager) StopAllProcesses() {
    pm.mu.RLock()
    names := make([]string, 0, len(pm.processes))
    for name := range pm.processes {
        names = append(names, name)
    }
    pm.mu.RUnlock()  // Release before calling StopProcess

    for _, name := range names {
        pm.StopProcess(name)
    }
}

---

4. Use Channels for Goroutine Communication

Description: Prefer channels over shared memory for goroutine coordination.

Search Pattern:

grep -rn "make(chan" cli/ --include="*.go"

Pass Criteria: Goroutines communicate via typed channels. Channel ownership is clear.

Fail Criteria: Goroutines share state through global variables without proper sync.

Severity: High

Recommendation:

func (m *chatModel) startStream() tea.Cmd {
    ch := make(chan tea.Msg, 32)  // Buffered for async
    m.streamCh = ch

    go func() {
        defer close(ch)  // Always close when done
        // Send messages through channel
        ch <- responseMsg{content: data}
    }()

    return listen(ch)
}

---

5. Buffer Channels Appropriately

Description: Choose buffer size based on producer/consumer patterns.

Search Pattern:

grep -rn "make(chan.*," cli/ --include="*.go"

Pass Criteria: Buffered channels used when producer shouldn't block. Unbuffered for synchronization.

Fail Criteria: Unbuffered channels causing deadlock. Oversized buffers wasting memory.

Severity: Medium

Recommendation:

// Buffered: producer shouldn't block on slow consumer
ch := make(chan tea.Msg, 32)

// Unbuffered: synchronization point needed
done := make(chan struct{})

---

6. Close Channels from Producer Side

Description: Only the channel producer should close the channel.

Search Pattern:

grep -rn "close(" cli/ --include="*.go"

Pass Criteria: Channels are closed by the goroutine that sends to them. Receivers never close.

Fail Criteria: Receivers closing channels, causing panic on send.

Severity: Critical

Recommendation:

go func() {
    defer close(ch)  // Producer closes
    for _, item := range items {
        ch <- item
    }
}()

// Receiver just reads
for msg := range ch {
    process(msg)
}

---

7. Use Context for Cancellation

Description: Use context.Context for timeout and cancellation propagation.

Search Pattern:

grep -rn "context\." cli/ --include="*.go"

Pass Criteria: Long-running operations accept context. Cancellation is respected.

Fail Criteria: Operations cannot be cancelled. Context ignored or not passed.

Severity: High

Recommendation:

func fetchSessionHistory(ctx context.Context, url string) (*History, error) {
    ctx, cancel := context.WithTimeout(ctx, 2*time.Second)
    defer cancel()

    req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
    if err != nil {
        return nil, err
    }
    // ...
}

---

8. Use sync.Once for One-Time Initialization

Description: Use sync.Once for thread-safe lazy initialization.

Search Pattern:

grep -rn "sync\.Once" cli/ --include="*.go"

Pass Criteria: Singleton initialization uses sync.Once. No race conditions.

Fail Criteria: Double-checked locking or other error-prone patterns.

Severity: Medium

Recommendation:

var (
    debugOnce   sync.Once
    debugLogger *log.Logger
)

func InitDebugLogger(path string) error {
    var initErr error
    debugOnce.Do(func() {
        // Initialization runs exactly once
        f, err := os.Create(path)
        if err != nil {
            initErr = err
            return
        }
        debugLogger = log.New(f, "", log.LstdFlags)
    })
    return initErr
}

---

9. Avoid Goroutine Leaks

Description: Every goroutine must have a clear exit condition.

Search Pattern:

grep -rn "go func" cli/ --include="*.go"

Pass Criteria: Goroutines have exit conditions (channel close, context cancel, timeout).

Fail Criteria: Goroutines block forever on channel reads or have no exit path.

Severity: High

Recommendation:

go func() {
    for {
        select {
        case msg, ok := <-ch:
            if !ok {
                return  // Channel closed, exit
            }
            process(msg)
        case <-ctx.Done():
            return  // Context cancelled, exit
        }
    }
}()

---

10. Use WaitGroup for Goroutine Coordination

Description: Use sync.WaitGroup to wait for multiple goroutines to complete.

Search Pattern:

grep -rn "sync\.WaitGroup" cli/ --include="*.go"

Pass Criteria: Parallel operations use WaitGroup. All goroutines are waited on.

Fail Criteria: Main goroutine exits before workers complete. Race conditions.

Severity: Medium

Recommendation:

func processAll(items []Item) {
    var wg sync.WaitGroup

    for _, item := range items {
        wg.Add(1)
        go func(it Item) {
            defer wg.Done()
            process(it)
        }(item)  // Pass item to avoid closure capture
    }

    wg.Wait()  // Block until all complete
}

Error Handling

Best practices for error creation, wrapping, and handling in the LlamaFarm CLI.

Checklist

1. Always Check Errors Immediately

Description: Check error return values immediately after function calls.

Search Pattern:

grep -rn ", err :=\|, err =\|, _ :=" cli/ --include="*.go" | head -50

Pass Criteria: Every error is checked. No ignored error returns (except intentionally).

Fail Criteria: Error returns ignored with _. Errors checked multiple lines later.

Severity: Critical

Recommendation:

// Good
f, err := os.Open(path)
if err != nil {
    return fmt.Errorf("failed to open file: %w", err)
}
defer f.Close()

// Bad - error ignored
f, _ := os.Open(path)  // NEVER do this

---

2. Wrap Errors with Context

Description: Wrap errors using fmt.Errorf with %w verb to add context.

Search Pattern:

grep -rn "fmt.Errorf.*%w" cli/ --include="*.go"

Pass Criteria: Errors are wrapped with context describing what operation failed.

Fail Criteria: Raw errors returned without context. Using %v instead of %w.

Severity: High

Recommendation:

func (pm *ProcessManager) StartProcess(name string) error {
    if err := cmd.Start(); err != nil {
        return fmt.Errorf("failed to start process %s: %w", name, err)
    }
    return nil
}

---

3. Use Sentinel Errors for Expected Conditions

Description: Define package-level sentinel errors for expected error conditions.

Search Pattern:

grep -rn "var Err.*= errors.New\|var Err.*= fmt.Errorf" cli/ --include="*.go"

Pass Criteria: Common error conditions have named sentinel errors. Callers can use errors.Is().

Fail Criteria: Stringly-typed error checking with string comparison.

Severity: Medium

Recommendation:

// Define sentinel errors
var ErrServiceAlreadyRunning = errors.New("service is already running")
var ErrProcessNotFound = errors.New("process not found")

// Use in code
if isRunning {
    return ErrServiceAlreadyRunning
}

// Check in caller
if errors.Is(err, ErrServiceAlreadyRunning) {
    // Handle expected case
}

---

4. Use errors.Is and errors.As for Checking

Description: Use errors.Is() and errors.As() instead of type assertions.

Search Pattern:

grep -rn "errors\.Is\|errors\.As" cli/ --include="*.go"

Pass Criteria: Error checking uses errors.Is() and errors.As() for wrapped errors.

Fail Criteria: Direct type assertions or string matching on error messages.

Severity: Medium

Recommendation:

// Check for specific error
if errors.Is(err, os.ErrNotExist) {
    // File doesn't exist
}

// Extract typed error
var healthErr *HealthError
if errors.As(err, &healthErr) {
    fmt.Printf("Server unhealthy: %s\n", healthErr.Status)
}

---

5. Create Custom Error Types When Needed

Description: Define custom error types for errors that carry additional context.

Search Pattern:

grep -rn "func.*Error().*string" cli/ --include="*.go"

Pass Criteria: Custom error types implement error interface. Carry relevant context.

Fail Criteria: Overuse of custom types. Context that could be in wrap message.

Severity: Low

Recommendation:

type HealthError struct {
    Status     string
    HealthResp HealthPayload
}

func (e *HealthError) Error() string {
    return fmt.Sprintf("server unhealthy: %s", e.Status)
}

// Usage
return &HealthError{Status: "degraded", HealthResp: payload}

---

6. Don't Log and Return

Description: Either log an error OR return it, not both.

Search Pattern:

grep -rn "log\|Log" -A2 cli/ --include="*.go" | grep "return.*err"

Pass Criteria: Errors are logged at the top level only. Lower levels just return.

Fail Criteria: Same error logged multiple times as it propagates up.

Severity: Medium

Recommendation:

// In library code - just return
func loadConfig() (*Config, error) {
    data, err := os.ReadFile(path)
    if err != nil {
        return nil, fmt.Errorf("read config: %w", err)
    }
    return parseConfig(data)
}

// At top level - log and handle
func Execute() {
    if err := rootCmd.Execute(); err != nil {
        fmt.Fprintf(os.Stderr, "Error: %v\n", err)
        os.Exit(1)
    }
}

---

7. Handle Errors Close to the Source

Description: Handle errors as close to where they occur as practical.

Search Pattern:

grep -rn "if err != nil" cli/ --include="*.go" | wc -l

Pass Criteria: Error handling happens immediately after the call. No distant error checks.

Fail Criteria: Errors stored and checked later. Complex error handling logic.

Severity: Medium

Recommendation:

// Good - handle immediately
resp, err := client.Do(req)
if err != nil {
    return nil, fmt.Errorf("request failed: %w", err)
}
defer resp.Body.Close()

// Bad - storing for later
var savedErr error
resp, savedErr = client.Do(req)
// ... other code ...
if savedErr != nil {  // Easy to forget
    return nil, savedErr
}

---

8. Use Early Returns

Description: Use early returns to handle errors and reduce nesting.

Search Pattern:

grep -rn "if err != nil {" -A3 cli/ --include="*.go" | grep "return"

Pass Criteria: Functions use early returns. Happy path is not deeply nested.

Fail Criteria: Deep nesting with else blocks. Complex control flow.

Severity: Medium

Recommendation:

// Good - early returns
func process(name string) error {
    if name == "" {
        return errors.New("name required")
    }

    data, err := load(name)
    if err != nil {
        return fmt.Errorf("load %s: %w", name, err)
    }

    return save(data)  // Happy path at end
}

// Bad - deep nesting
func process(name string) error {
    if name != "" {
        data, err := load(name)
        if err == nil {
            // Happy path deeply nested
        }
    }
}

---

9. Provide Actionable Error Messages

Description: Error messages should help users understand what to do.

Search Pattern:

grep -rn "fmt.Errorf\|errors.New" cli/ --include="*.go"

Pass Criteria: Error messages describe the problem and suggest resolution.

Fail Criteria: Cryptic error messages. Technical jargon without context.

Severity: Medium

Recommendation:

// Good - actionable
return fmt.Errorf("service %s failed to start. Run 'lf services logs -s %s' to view logs", name, name)

// Bad - not actionable
return errors.New("start failed")

---

10. Use Panic Only for Programming Errors

Description: Reserve panic for unrecoverable programming errors, not runtime errors.

Search Pattern:

grep -rn "panic(" cli/ --include="*.go"

Pass Criteria: Panic used only for invariant violations, nil pointer protection, or init failures.

Fail Criteria: Panic used for expected runtime errors like file not found.

Severity: High

Recommendation:

// Acceptable - programming error
func MustParse(s string) *Config {
    cfg, err := Parse(s)
    if err != nil {
        panic(fmt.Sprintf("invalid config: %v", err))
    }
    return cfg
}

// Bad - runtime error
func ReadFile(path string) []byte {
    data, err := os.ReadFile(path)
    if err != nil {
        panic(err)  // NEVER do this
    }
    return data
}

Idiomatic Go Patterns

Best practices for writing idiomatic Go code in the LlamaFarm CLI.

Checklist

1. Use Named Return Values Sparingly

Description: Named return values should only be used when they add clarity to documentation or enable deferred error handling.

Search Pattern:

grep -rn "func.*\(.*\).*\(.*,.*\)" cli/ --include="*.go" | grep -v "_test.go"

Pass Criteria: Named returns are used for documentation or defer patterns, not just convenience.

Fail Criteria: Named returns used unnecessarily, leading to confusing code with naked returns.

Severity: Low

Recommendation: Remove named returns unless they serve documentation or defer purposes. Use explicit returns.

---

2. Accept Interfaces, Return Structs

Description: Functions should accept interfaces for flexibility and return concrete types for clarity.

Search Pattern:

grep -rn "func.*interface{}" cli/ --include="*.go"

Pass Criteria: Functions accept narrow interfaces (e.g., io.Reader) and return concrete structs.

Fail Criteria: Functions return interfaces or accept overly broad interfaces like interface{}.

Severity: Medium

Recommendation: Define small, focused interfaces. Return concrete types. Example:

// Good
type HTTPClient interface {
    Do(req *http.Request) (*http.Response, error)
}

func NewClient() *DefaultHTTPClient { ... }

// Avoid
func NewClient() HTTPClient { ... }  // Returns interface

---

3. Use Constructor Functions

Description: Complex structs should have constructor functions that validate and initialize.

Search Pattern:

grep -rn "func New" cli/ --include="*.go"

Pass Criteria: Constructors validate inputs, set defaults, and return errors when appropriate.

Fail Criteria: Struct initialization scattered throughout code without validation.

Severity: Medium

Recommendation: Use NewXxx pattern:

func NewProcessManager() (*ProcessManager, error) {
    lfDataDir, err := utils.GetLFDataDir()
    if err != nil {
        return nil, fmt.Errorf("failed to get LF data directory: %w", err)
    }
    // ... initialization
    return &ProcessManager{...}, nil
}

---

4. Use Functional Options for Configuration

Description: For structs with many optional configuration parameters, use functional options.

Search Pattern:

grep -rn "type.*Option.*func" cli/ --include="*.go"

Pass Criteria: Complex configuration uses functional options pattern for clarity and extensibility.

Fail Criteria: Long constructor parameter lists or excessive struct field exposure.

Severity: Low

Recommendation:

type Option func(*Config)

func WithTimeout(d time.Duration) Option {
    return func(c *Config) { c.Timeout = d }
}

func NewClient(opts ...Option) *Client {
    cfg := defaultConfig()
    for _, opt := range opts {
        opt(&cfg)
    }
    return &Client{cfg: cfg}
}

---

5. Embed for Composition

Description: Use struct embedding for composition instead of inheritance-like patterns.

Search Pattern:

grep -rn "type.*struct {$" -A5 cli/ --include="*.go" | grep -E "^\s+\*?[A-Z]"

Pass Criteria: Embedding used to share behavior (e.g., embedding sync.Mutex).

Fail Criteria: Deep inheritance hierarchies or excessive embedding that obscures behavior.

Severity: Low

Recommendation:

type ProcessInfo struct {
    Name    string
    Cmd     *exec.Cmd
    mu      sync.RWMutex  // Embedded for locking
}

---

6. Use defer for Cleanup

Description: Use defer for resource cleanup to ensure cleanup happens even on error paths.

Search Pattern:

grep -rn "defer" cli/ --include="*.go"

Pass Criteria: All file handles, locks, and connections use defer for cleanup.

Fail Criteria: Manual cleanup at multiple return points, risk of resource leaks.

Severity: High

Recommendation:

func readFile(path string) ([]byte, error) {
    f, err := os.Open(path)
    if err != nil {
        return nil, err
    }
    defer f.Close()  // Always executes

    return io.ReadAll(f)
}

---

7. Keep Functions Small and Focused

Description: Functions should do one thing well. Extract helper functions for complex logic.

Search Pattern:

# Find functions longer than 50 lines
awk '/^func /{start=NR; name=$0} /^}$/ && start{if(NR-start>50) print FILENAME":"start": "name}' cli/**/*.go

Pass Criteria: Most functions are under 50 lines. Complex logic is extracted into helpers.

Fail Criteria: Monolithic functions with deeply nested logic.

Severity: Medium

Recommendation: Extract logical blocks into well-named helper functions. Use early returns.

---

8. Use Constants for Magic Values

Description: Define constants for repeated values and configuration defaults.

Search Pattern:

grep -rn "const (" cli/ --include="*.go"

Pass Criteria: Timeouts, buffer sizes, and configuration values are defined as constants.

Fail Criteria: Magic numbers scattered throughout code.

Severity: Medium

Recommendation:

const (
    ServiceLockTimeout     = 30 * time.Second
    ServiceLockPollInterval = 500 * time.Millisecond
    PIDFileWaitTimeout     = 10 * time.Second
)

---

9. Use Type Aliases for Clarity

Description: Define type aliases to add semantic meaning to primitive types.

Search Pattern:

grep -rn "^type.*int$\|^type.*string$" cli/ --include="*.go"

Pass Criteria: Domain-specific types like SessionMode or ServiceStatus are defined.

Fail Criteria: Raw primitives used everywhere without semantic context.

Severity: Low

Recommendation:

type SessionMode int

const (
    SessionModeProject SessionMode = iota
    SessionModeStateless
    SessionModeDev
)

---

10. Avoid Package-Level State

Description: Minimize package-level variables. Prefer dependency injection.

Search Pattern:

grep -rn "^var " cli/ --include="*.go" | grep -v "_test.go"

Pass Criteria: Package-level state is limited to singletons (like rootCmd) or configuration.

Fail Criteria: Mutable package-level state that makes testing difficult.

Severity: Medium

Recommendation: Pass dependencies explicitly through constructors or function parameters. Use package-level vars only for immutable constants or required singletons.

Security Patterns

Best practices for secure coding in the LlamaFarm CLI.

Checklist

1. Never Log Credentials or Tokens

Description: Sensitive data (API keys, tokens, passwords) must never appear in logs.

Search Pattern:

grep -rn "LogDebug\|log\.Print\|fmt\.Print" cli/ --include="*.go" | grep -i "token\|password\|secret\|key\|auth"

Pass Criteria: Sensitive values are redacted before logging. Sanitization applied.

Fail Criteria: Credentials visible in debug logs. Tokens printed to stderr.

Severity: Critical

Recommendation:

// Use sanitization for all logs
func LogDebug(msg string) {
    sanitized := sanitizeLogMessage(msg)
    debugLogger.Println(sanitized)
}

// Redact headers explicitly
func LogHeaders(kind string, hdr http.Header) {
    sensitiveHeaders := map[string]struct{}{
        "authorization": {},
        "cookie":        {},
        "x-api-key":     {},
    }
    for k, vals := range hdr {
        if _, sensitive := sensitiveHeaders[strings.ToLower(k)]; sensitive {
            LogDebug(fmt.Sprintf("%s header: %s: [REDACTED]", kind, k))
        } else {
            LogDebug(fmt.Sprintf("%s header: %s: %s", kind, k, vals))
        }
    }
}

---

2. Use Regex Sanitization for Logs

Description: Apply regex patterns to automatically redact sensitive data in logs.

Search Pattern:

grep -rn "regexp\|Regexp" cli/ --include="*.go"

Pass Criteria: Log sanitization catches JWTs, API keys, passwords, and other secrets.

Fail Criteria: Manual redaction only. Patterns miss common credential formats.

Severity: High

Recommendation:

var sensitivePatterns = []struct {
    pattern     *regexp.Regexp
    replacement string
}{
    // JWT tokens
    {regexp.MustCompile(`\beyJ[a-zA-Z0-9_-]+\.eyJ[a-zA-Z0-9_-]+\.[a-zA-Z0-9_-]+`), "[REDACTED-JWT]"},
    // API keys
    {regexp.MustCompile(`\b(sk|pk|sess)-[a-zA-Z0-9\-_]{20,}`), "[REDACTED-KEY]"},
    // Bearer tokens
    {regexp.MustCompile(`(?i)(bearer\s+)[a-zA-Z0-9\-_\.]+`), "${1}[REDACTED]"},
    // Passwords
    {regexp.MustCompile(`(?i)(password[=:\s]+['"]?)[^\s&'"]+`), "${1}[REDACTED]"},
}

func sanitizeLogMessage(msg string) string {
    sanitized := msg
    for _, sp := range sensitivePatterns {
        sanitized = sp.pattern.ReplaceAllString(sanitized, sp.replacement)
    }
    return sanitized
}

---

3. Validate All External Input

Description: Validate input from users, files, and network before use.

Search Pattern:

grep -rn "os\.Args\|flag\.\|cobra\.\|args\[" cli/ --include="*.go"

Pass Criteria: All user input validated. Length limits enforced. Invalid input rejected.

Fail Criteria: Unchecked input passed to system calls. No validation on file paths.

Severity: Critical

Recommendation:

func processFile(path string) error {
    // Validate path
    if path == "" {
        return errors.New("path required")
    }

    // Clean path to prevent traversal
    cleanPath := filepath.Clean(path)

    // Check within allowed directory
    if !strings.HasPrefix(cleanPath, allowedDir) {
        return errors.New("path outside allowed directory")
    }

    // Check file exists and is regular file
    info, err := os.Stat(cleanPath)
    if err != nil {
        return fmt.Errorf("cannot access file: %w", err)
    }
    if !info.Mode().IsRegular() {
        return errors.New("not a regular file")
    }

    return nil
}

---

4. Use Context for Request Timeouts

Description: All HTTP requests must have timeouts to prevent resource exhaustion.

Search Pattern:

grep -rn "http\.NewRequest\|http\.Get\|http\.Post" cli/ --include="*.go"

Pass Criteria: All HTTP requests use context with timeout. No indefinite waits.

Fail Criteria: Requests without timeout. Default http.Client used.

Severity: High

Recommendation:

func fetchData(url string) ([]byte, error) {
    ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
    defer cancel()

    req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
    if err != nil {
        return nil, err
    }

    // Also set client timeout as backup
    client := &http.Client{Timeout: 12 * time.Second}
    resp, err := client.Do(req)
    if err != nil {
        return nil, err
    }
    defer resp.Body.Close()

    return io.ReadAll(resp.Body)
}

---

5. Limit Response Body Size

Description: Limit the size of data read from external sources.

Search Pattern:

grep -rn "io\.ReadAll\|ioutil\.ReadAll" cli/ --include="*.go"

Pass Criteria: Response body reads have size limits. Large bodies handled as streams.

Fail Criteria: Unbounded reads that could exhaust memory.

Severity: Medium

Recommendation:

const maxBodySize = 10 * 1024 * 1024  // 10MB

func readBody(resp *http.Response) ([]byte, error) {
    // Limit reader prevents memory exhaustion
    limitedReader := io.LimitReader(resp.Body, maxBodySize)
    data, err := io.ReadAll(limitedReader)
    if err != nil {
        return nil, err
    }

    if int64(len(data)) == maxBodySize {
        return nil, errors.New("response body too large")
    }

    return data, nil
}

---

6. Secure File Permissions

Description: Create files with restrictive permissions. Never world-writable.

Search Pattern:

grep -rn "os\.Create\|os\.WriteFile\|os\.OpenFile\|0777\|0666" cli/ --include="*.go"

Pass Criteria: Files created with 0644 or more restrictive. Directories with 0755.

Fail Criteria: World-writable files (0666, 0777). Sensitive data in world-readable files.

Severity: High

Recommendation:

// Config files - owner read/write only
if err := os.WriteFile(path, data, 0600); err != nil {
    return err
}

// Log files - owner read/write, group/other read
if err := os.WriteFile(logPath, data, 0644); err != nil {
    return err
}

// Directories - owner full, group/other read/execute
if err := os.MkdirAll(dir, 0755); err != nil {
    return err
}

// PID files - restrictive
if err := os.WriteFile(pidPath, []byte(pid), 0644); err != nil {
    return err
}

---

7. Shell Escape Arguments

Description: When building shell commands, properly escape arguments.

Search Pattern:

grep -rn "exec\.Command\|fmt\.Sprintf.*shell\|strings\.Builder" cli/ --include="*.go"

Pass Criteria: Shell arguments are escaped. No command injection possible.

Fail Criteria: User input directly in shell commands. Unescaped special characters.

Severity: Critical

Recommendation:

// Use exec.Command with separate arguments (safe)
cmd := exec.Command("git", "commit", "-m", message)

// When building shell strings for display, escape properly
func shellEscapeSingleQuotes(s string) string {
    return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'"
}

// For curl commands shown to users
func buildCurlCommand(url string, body []byte) string {
    var b strings.Builder
    b.WriteString("curl -X POST ")
    b.WriteString("-d ")
    b.WriteString(shellEscapeSingleQuotes(string(body)))
    b.WriteString(" ")
    b.WriteString(shellEscapeSingleQuotes(url))
    return b.String()
}

---

8. Avoid Path Traversal

Description: Validate file paths to prevent directory traversal attacks.

Search Pattern:

grep -rn "filepath\.Join\|filepath\.Clean\|os\.Open\|os\.ReadFile" cli/ --include="*.go"

Pass Criteria: All paths cleaned with filepath.Clean. Paths validated against base directory.

Fail Criteria: User-controlled paths used directly. ../ sequences possible.

Severity: Critical

Recommendation:

func safeReadFile(baseDir, userPath string) ([]byte, error) {
    // Clean the user-provided path
    cleanPath := filepath.Clean(userPath)

    // Remove any leading path separators
    cleanPath = strings.TrimPrefix(cleanPath, string(filepath.Separator))

    // Join with base directory
    fullPath := filepath.Join(baseDir, cleanPath)

    // Verify result is still within base directory
    if !strings.HasPrefix(fullPath, filepath.Clean(baseDir)+string(filepath.Separator)) {
        return nil, errors.New("path escapes base directory")
    }

    return os.ReadFile(fullPath)
}

---

9. Handle Signals Gracefully

Description: Handle OS signals for graceful shutdown and cleanup.

Search Pattern:

grep -rn "os\.Signal\|signal\.Notify\|syscall\." cli/ --include="*.go"

Pass Criteria: SIGINT and SIGTERM handled. Resources cleaned up on shutdown.

Fail Criteria: Abrupt termination. Resources left in inconsistent state.

Severity: Medium

Recommendation:

func setupSignalHandler(cleanup func()) {
    sigCh := make(chan os.Signal, 1)
    signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)

    go func() {
        <-sigCh
        fmt.Println("\nShutting down gracefully...")
        cleanup()
        os.Exit(0)
    }()
}

// Usage
func main() {
    setupSignalHandler(func() {
        orchestrator.StopAllProcesses()
        utils.CloseDebugLogger()
    })

    cmd.Execute()
}

---

10. Redact Sensitive Data in Error Messages

Description: Error messages should not expose sensitive data.

Search Pattern:

grep -rn "fmt\.Errorf\|errors\.New" cli/ --include="*.go" | grep -i "token\|password\|key"

Pass Criteria: Error messages describe the problem without exposing secrets.

Fail Criteria: Passwords or tokens visible in error output.

Severity: High

Recommendation:

// Bad - exposes token
return fmt.Errorf("authentication failed with token: %s", token)

// Good - describes problem without exposing secret
return fmt.Errorf("authentication failed: invalid or expired token")

// Good - includes request ID for debugging without secrets
return fmt.Errorf("API request failed (request_id=%s): %w", requestID, err)

Testing Patterns

Best practices for writing tests in the LlamaFarm CLI.

Checklist

1. Use Table-Driven Tests

Description: Structure tests as tables of inputs and expected outputs.

Search Pattern:

grep -rn "tests := \[\]struct\|tt := range tests" cli/ --include="*_test.go"

Pass Criteria: Complex test scenarios use table-driven pattern. Easy to add cases.

Fail Criteria: Repetitive test code. Hard to add new test cases.

Severity: Medium

Recommendation:

func TestResolveDependencies(t *testing.T) {
    tests := []struct {
        name        string
        serviceName string
        wantOrder   []string
        wantErr     bool
        errContains string
    }{
        {
            name:        "resolve server with no dependencies",
            serviceName: "server",
            wantOrder:   []string{"server"},
            wantErr:     false,
        },
        {
            name:        "unknown service returns error",
            serviceName: "unknown-service",
            wantErr:     true,
            errContains: "unknown service",
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            got, err := sm.resolveDependencies(tt.serviceName)
            // assertions...
        })
    }
}

---

2. Use t.Run for Subtests

Description: Use t.Run() to create named subtests for better output and isolation.

Search Pattern:

grep -rn "t\.Run(" cli/ --include="*_test.go"

Pass Criteria: All table-driven tests use t.Run(). Subtests have descriptive names.

Fail Criteria: Tests without subtests. Unclear which case failed.

Severity: Medium

Recommendation:

for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        // Each subtest runs in isolation
        // Can use t.Parallel() for concurrent execution
    })
}

---

3. Use Interfaces for Mocking

Description: Define interfaces for external dependencies to enable mocking.

Search Pattern:

grep -rn "type.*interface {" cli/ --include="*.go"

Pass Criteria: External dependencies (HTTP, filesystem) accessed through interfaces.

Fail Criteria: Direct use of concrete types. Cannot test without real dependencies.

Severity: High

Recommendation:

// Define interface
type HTTPClient interface {
    Do(req *http.Request) (*http.Response, error)
}

// Use in production
var httpClient HTTPClient = &DefaultHTTPClient{Timeout: 60 * time.Second}

// Swap for testing
func SetHTTPClientForTest(client HTTPClient) {
    httpClient = client
}

// Mock in tests
type mockHTTPClient struct {
    response *http.Response
    err      error
}

func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) {
    return m.response, m.err
}

---

4. Test Both Success and Error Cases

Description: Every test should verify both happy path and error conditions.

Search Pattern:

grep -rn "wantErr" cli/ --include="*_test.go"

Pass Criteria: Tests include error cases. Both wantErr: true and wantErr: false present.

Fail Criteria: Only happy path tested. Error handling untested.

Severity: High

Recommendation:

tests := []struct {
    name    string
    input   string
    want    *Result
    wantErr bool
}{
    {"valid input", "good", &Result{...}, false},
    {"empty input", "", nil, true},
    {"invalid format", "bad", nil, true},
}

---

5. Use t.TempDir for Temporary Files

Description: Use t.TempDir() for tests that need temporary directories.

Search Pattern:

grep -rn "t\.TempDir\|os\.MkdirTemp" cli/ --include="*_test.go"

Pass Criteria: Tests use t.TempDir() for automatic cleanup. No temp file leaks.

Fail Criteria: Manual temp directory creation. Cleanup in defer forgotten.

Severity: Medium

Recommendation:

func TestSessionPersistence(t *testing.T) {
    // t.TempDir() automatically cleans up
    tempDir := t.TempDir()

    // Or for Go 1.14 compatibility
    tempDir, err := os.MkdirTemp("", "test")
    if err != nil {
        t.Fatalf("failed to create temp dir: %v", err)
    }
    defer os.RemoveAll(tempDir)
}

---

6. Use t.Cleanup for Resource Cleanup

Description: Use t.Cleanup() for cleanup that must run after test completion.

Search Pattern:

grep -rn "t\.Cleanup\|defer.*t\." cli/ --include="*_test.go"

Pass Criteria: Resources registered with t.Cleanup(). Cleanup runs even on failure.

Fail Criteria: Cleanup in defer that doesn't run on fatal. Resource leaks.

Severity: Medium

Recommendation:

func TestWithEnvVar(t *testing.T) {
    orig := os.Getenv("MY_VAR")
    os.Setenv("MY_VAR", "test-value")

    t.Cleanup(func() {
        if orig != "" {
            os.Setenv("MY_VAR", orig)
        } else {
            os.Unsetenv("MY_VAR")
        }
    })

    // Test code...
}

---

7. Use t.Helper for Test Helpers

Description: Mark test helper functions with t.Helper() for better error reporting.

Search Pattern:

grep -rn "t\.Helper()" cli/ --include="*_test.go"

Pass Criteria: Helper functions call t.Helper(). Errors point to actual test line.

Fail Criteria: Errors point to helper function instead of failing test.

Severity: Low

Recommendation:

func assertNoError(t *testing.T, err error) {
    t.Helper()  // Error will point to caller, not this line
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
}

func assertEqual(t *testing.T, got, want interface{}) {
    t.Helper()
    if got != want {
        t.Errorf("got %v, want %v", got, want)
    }
}

---

8. Test File Naming Convention

Description: Test files should be named *_test.go in the same package.

Search Pattern:

find cli/ -name "*_test.go" -type f

Pass Criteria: Test files follow xxx_test.go pattern. Located with source files.

Fail Criteria: Tests in separate directory. Non-standard naming.

Severity: Low

Recommendation:

cli/cmd/
  chat_client.go
  chat_client_test.go    # Same package, same directory
  orchestrator/
    services.go
    services_test.go     # Same package, same directory

---

9. Use Parallel Tests When Safe

Description: Use t.Parallel() for tests that don't share state.

Search Pattern:

grep -rn "t\.Parallel()" cli/ --include="*_test.go"

Pass Criteria: Independent tests run in parallel. Test suite completes faster.

Fail Criteria: Tests with shared state run in parallel causing flakes.

Severity: Low

Recommendation:

func TestSomething(t *testing.T) {
    tests := []struct{...}

    for _, tt := range tests {
        tt := tt  // Capture range variable
        t.Run(tt.name, func(t *testing.T) {
            t.Parallel()  // Safe if no shared state
            // Test code...
        })
    }
}

---

10. Verify Error Messages

Description: When testing error cases, verify the error message content.

Search Pattern:

grep -rn "errContains\|strings.Contains.*err" cli/ --include="*_test.go"

Pass Criteria: Error tests verify error message contains expected text.

Fail Criteria: Only checking err != nil. Wrong error type could pass.

Severity: Medium

Recommendation:

func TestErrorMessages(t *testing.T) {
    tests := []struct {
        name        string
        input       string
        errContains string
    }{
        {"missing file", "nonexistent", "no such file"},
        {"invalid format", "bad.txt", "invalid format"},
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            _, err := process(tt.input)
            if err == nil {
                t.Fatal("expected error")
            }
            if !strings.Contains(err.Error(), tt.errContains) {
                t.Errorf("error %q should contain %q", err, tt.errContains)
            }
        })
    }
}