diff --git a/Taskfile.yml b/Taskfile.yml index de548953..d4379901 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -3,20 +3,35 @@ version: '3' vars: VERSION: sh: git describe --tags --exact-match 2>/dev/null || echo "dev" - LDFLAGS: "-X github.com/host-uk/core/pkg/cli.AppVersion={{.VERSION}}" + # Base ldflags for version injection + LDFLAGS_BASE: "-X github.com/host-uk/core/pkg/cli.AppVersion={{.VERSION}}" + # Development build: includes debug info + LDFLAGS: "{{.LDFLAGS_BASE}}" + # Release build: strips debug info and symbol table for smaller binary + LDFLAGS_RELEASE: "-s -w {{.LDFLAGS_BASE}}" tasks: # --- CLI Management --- cli:build: - desc: "Build core CLI to ./bin/core" + desc: "Build core CLI to ./bin/core (dev build with debug info)" cmds: - go build -ldflags '{{.LDFLAGS}}' -o ./bin/core . + cli:build:release: + desc: "Build core CLI for release (smaller binary, no debug info)" + cmds: + - go build -ldflags '{{.LDFLAGS_RELEASE}}' -o ./bin/core . + cli:install: - desc: "Install core CLI to system PATH" + desc: "Install core CLI to system PATH (dev build)" cmds: - go install -ldflags '{{.LDFLAGS}}' . + cli:install:release: + desc: "Install core CLI for release (smaller binary)" + cmds: + - go install -ldflags '{{.LDFLAGS_RELEASE}}' . + # --- Development --- test: desc: "Run all tests" diff --git a/internal/cmd/dev/cmd_file_sync.go b/internal/cmd/dev/cmd_file_sync.go index 4886683a..9eb44fbc 100644 --- a/internal/cmd/dev/cmd_file_sync.go +++ b/internal/cmd/dev/cmd_file_sync.go @@ -15,10 +15,10 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/git" "github.com/host-uk/core/pkg/i18n" coreio "github.com/host-uk/core/pkg/io" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -59,7 +59,7 @@ func runFileSync(source string) error { // Security: Reject path traversal attempts if strings.Contains(source, "..") { - return errors.E("dev.sync", "path traversal not allowed", nil) + return log.E("dev.sync", "path traversal not allowed", nil) } // Validate source exists @@ -82,7 +82,7 @@ func runFileSync(source string) error { // Let's stick to os.Stat for source properties finding as typically allowed for CLI args. if err != nil { - return errors.E("dev.sync", i18n.T("cmd.dev.file_sync.error.source_not_found", map[string]interface{}{"Path": source}), err) + return log.E("dev.sync", i18n.T("cmd.dev.file_sync.error.source_not_found", map[string]interface{}{"Path": source}), err) } // Find target repos @@ -131,7 +131,11 @@ func runFileSync(source string) error { } } else { // Ensure dir exists - coreio.Local.EnsureDir(filepath.Dir(destPath)) + if err := coreio.Local.EnsureDir(filepath.Dir(destPath)); err != nil { + cli.Print(" %s %s: copy failed: %s\n", errorStyle.Render("x"), repoName, err) + failed++ + continue + } if err := coreio.Copy(coreio.Local, source, coreio.Local, destPath); err != nil { cli.Print(" %s %s: copy failed: %s\n", errorStyle.Render("x"), repoName, err) failed++ @@ -205,12 +209,12 @@ func resolveTargetRepos(pattern string) ([]*repos.Repo, error) { // Load registry registryPath, err := repos.FindRegistry() if err != nil { - return nil, errors.E("dev.sync", "failed to find registry", err) + return nil, log.E("dev.sync", "failed to find registry", err) } registry, err := repos.LoadRegistry(registryPath) if err != nil { - return nil, errors.E("dev.sync", "failed to load registry", err) + return nil, log.E("dev.sync", "failed to load registry", err) } // Match pattern against repo names diff --git a/internal/cmd/docs/cmd_sync.go b/internal/cmd/docs/cmd_sync.go index a1611056..d7799ac7 100644 --- a/internal/cmd/docs/cmd_sync.go +++ b/internal/cmd/docs/cmd_sync.go @@ -140,7 +140,10 @@ func runDocsSync(registryPath string, outputDir string, dryRun bool) error { src := filepath.Join(docsDir, f) dst := filepath.Join(repoOutDir, f) // Ensure parent dir - io.Local.EnsureDir(filepath.Dir(dst)) + if err := io.Local.EnsureDir(filepath.Dir(dst)); err != nil { + cli.Print(" %s %s: %s\n", errorStyle.Render("✗"), f, err) + continue + } if err := io.Copy(io.Local, src, io.Local, dst); err != nil { cli.Print(" %s %s: %s\n", errorStyle.Render("✗"), f, err) diff --git a/internal/cmd/monitor/cmd_monitor.go b/internal/cmd/monitor/cmd_monitor.go index 62f4b680..847fc991 100644 --- a/internal/cmd/monitor/cmd_monitor.go +++ b/internal/cmd/monitor/cmd_monitor.go @@ -17,8 +17,8 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -107,7 +107,7 @@ type SecretScanningAlert struct { func runMonitor() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("monitor", i18n.T("error.gh_not_found"), err) + return log.E("monitor", i18n.T("error.gh_not_found"), err) } // Determine repos to scan @@ -117,7 +117,7 @@ func runMonitor() error { } if len(repoList) == 0 { - return errors.E("monitor", i18n.T("cmd.monitor.error.no_repos"), nil) + return log.E("monitor", i18n.T("cmd.monitor.error.no_repos"), nil) } // Collect all findings and errors @@ -179,12 +179,12 @@ func resolveRepos() ([]string, error) { // All repos from registry registry, err := repos.FindRegistry() if err != nil { - return nil, errors.E("monitor", "failed to find registry", err) + return nil, log.E("monitor", "failed to find registry", err) } loaded, err := repos.LoadRegistry(registry) if err != nil { - return nil, errors.E("monitor", "failed to load registry", err) + return nil, log.E("monitor", "failed to load registry", err) } var repoList []string @@ -253,12 +253,12 @@ func fetchCodeScanningAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchCodeScanning", "API request failed", err) + return nil, log.E("monitor.fetchCodeScanning", "API request failed", err) } var alerts []CodeScanningAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchCodeScanning", "failed to parse response", err) + return nil, log.E("monitor.fetchCodeScanning", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -307,12 +307,12 @@ func fetchDependabotAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchDependabot", "API request failed", err) + return nil, log.E("monitor.fetchDependabot", "API request failed", err) } var alerts []DependabotAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchDependabot", "failed to parse response", err) + return nil, log.E("monitor.fetchDependabot", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -358,12 +358,12 @@ func fetchSecretScanningAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchSecretScanning", "API request failed", err) + return nil, log.E("monitor.fetchSecretScanning", "API request failed", err) } var alerts []SecretScanningAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchSecretScanning", "failed to parse response", err) + return nil, log.E("monitor.fetchSecretScanning", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -447,7 +447,7 @@ func sortBySeverity(findings []Finding) { func outputJSON(findings []Finding) error { data, err := json.MarshalIndent(findings, "", " ") if err != nil { - return errors.E("monitor", "failed to marshal findings", err) + return log.E("monitor", "failed to marshal findings", err) } cli.Print("%s\n", string(data)) return nil @@ -547,7 +547,7 @@ func detectRepoFromGit() (string, error) { cmd := exec.Command("git", "remote", "get-url", "origin") output, err := cmd.Output() if err != nil { - return "", errors.E("monitor", i18n.T("cmd.monitor.error.not_git_repo"), err) + return "", log.E("monitor", i18n.T("cmd.monitor.error.not_git_repo"), err) } url := strings.TrimSpace(string(output)) diff --git a/internal/cmd/qa/cmd_health.go b/internal/cmd/qa/cmd_health.go index 95dca54c..f34cee3d 100644 --- a/internal/cmd/qa/cmd_health.go +++ b/internal/cmd/qa/cmd_health.go @@ -13,8 +13,8 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -63,7 +63,7 @@ func addHealthCommand(parent *cli.Command) { func runHealth() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.health", i18n.T("error.gh_not_found"), nil) + return log.E("qa.health", i18n.T("error.gh_not_found"), nil) } // Load registry @@ -75,12 +75,12 @@ func runHealth() error { } else { registryPath, findErr := repos.FindRegistry() if findErr != nil { - return errors.E("qa.health", i18n.T("error.registry_not_found"), nil) + return log.E("qa.health", i18n.T("error.registry_not_found"), nil) } reg, err = repos.LoadRegistry(registryPath) } if err != nil { - return errors.E("qa.health", "failed to load registry", err) + return log.E("qa.health", "failed to load registry", err) } // Fetch CI status from all repos diff --git a/internal/cmd/qa/cmd_issues.go b/internal/cmd/qa/cmd_issues.go index d243fc03..2b038c6d 100644 --- a/internal/cmd/qa/cmd_issues.go +++ b/internal/cmd/qa/cmd_issues.go @@ -16,8 +16,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -92,7 +92,7 @@ func addIssuesCommand(parent *cli.Command) { func runQAIssues() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.issues", i18n.T("error.gh_not_found"), nil) + return log.E("qa.issues", i18n.T("error.gh_not_found"), nil) } // Load registry @@ -104,12 +104,12 @@ func runQAIssues() error { } else { registryPath, findErr := repos.FindRegistry() if findErr != nil { - return errors.E("qa.issues", i18n.T("error.registry_not_found"), nil) + return log.E("qa.issues", i18n.T("error.registry_not_found"), nil) } reg, err = repos.LoadRegistry(registryPath) } if err != nil { - return errors.E("qa.issues", "failed to load registry", err) + return log.E("qa.issues", "failed to load registry", err) } // Fetch issues from all repos diff --git a/internal/cmd/qa/cmd_review.go b/internal/cmd/qa/cmd_review.go index 30945856..7bae5e47 100644 --- a/internal/cmd/qa/cmd_review.go +++ b/internal/cmd/qa/cmd_review.go @@ -16,8 +16,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" ) // Review command flags @@ -102,7 +102,7 @@ func addReviewCommand(parent *cli.Command) { func runReview() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.review", i18n.T("error.gh_not_found"), nil) + return log.E("qa.review", i18n.T("error.gh_not_found"), nil) } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -114,7 +114,7 @@ func runReview() error { var err error repoFullName, err = detectRepoFromGit() if err != nil { - return errors.E("qa.review", i18n.T("cmd.qa.review.error.no_repo"), nil) + return log.E("qa.review", i18n.T("cmd.qa.review.error.no_repo"), nil) } } @@ -144,7 +144,7 @@ func runReview() error { func showMyPRs(ctx context.Context, repo string) error { prs, err := fetchPRs(ctx, repo, "author:@me") if err != nil { - return errors.E("qa.review", "failed to fetch your PRs", err) + return log.E("qa.review", "failed to fetch your PRs", err) } if len(prs) == 0 { @@ -165,7 +165,7 @@ func showMyPRs(ctx context.Context, repo string) error { func showRequestedReviews(ctx context.Context, repo string) error { prs, err := fetchPRs(ctx, repo, "review-requested:@me") if err != nil { - return errors.E("qa.review", "failed to fetch review requests", err) + return log.E("qa.review", "failed to fetch review requests", err) } if len(prs) == 0 { diff --git a/internal/cmd/qa/cmd_watch.go b/internal/cmd/qa/cmd_watch.go index 2db17fe3..38ec20d7 100644 --- a/internal/cmd/qa/cmd_watch.go +++ b/internal/cmd/qa/cmd_watch.go @@ -17,8 +17,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" ) // Watch command flags @@ -79,7 +79,7 @@ func addWatchCommand(parent *cli.Command) { func runWatch() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.watch", i18n.T("error.gh_not_found"), nil) + return log.E("qa.watch", i18n.T("error.gh_not_found"), nil) } // Determine repo @@ -115,12 +115,12 @@ func runWatch() error { // Check if context deadline exceeded if ctx.Err() != nil { cli.Blank() - return errors.E("qa.watch", i18n.T("cmd.qa.watch.timeout", map[string]interface{}{"Duration": watchTimeout}), nil) + return log.E("qa.watch", i18n.T("cmd.qa.watch.timeout", map[string]interface{}{"Duration": watchTimeout}), nil) } runs, err := fetchWorkflowRunsForCommit(ctx, repoFullName, commitSha) if err != nil { - return errors.Wrap(err, "qa.watch", "failed to fetch workflow runs") + return log.Wrap(err, "qa.watch", "failed to fetch workflow runs") } if len(runs) == 0 { @@ -195,7 +195,7 @@ func resolveRepo(specified string) (string, error) { if org != "" { return org + "/" + specified, nil } - return "", errors.E("qa.watch", i18n.T("cmd.qa.watch.error.repo_format"), nil) + return "", log.E("qa.watch", i18n.T("cmd.qa.watch.error.repo_format"), nil) } // Detect from current directory @@ -212,7 +212,7 @@ func resolveCommit(specified string) (string, error) { cmd := exec.Command("git", "rev-parse", "HEAD") output, err := cmd.Output() if err != nil { - return "", errors.Wrap(err, "qa.watch", "failed to get HEAD commit") + return "", log.Wrap(err, "qa.watch", "failed to get HEAD commit") } return strings.TrimSpace(string(output)), nil @@ -223,7 +223,7 @@ func detectRepoFromGit() (string, error) { cmd := exec.Command("git", "remote", "get-url", "origin") output, err := cmd.Output() if err != nil { - return "", errors.E("qa.watch", i18n.T("cmd.qa.watch.error.not_git_repo"), nil) + return "", log.E("qa.watch", i18n.T("cmd.qa.watch.error.not_git_repo"), nil) } url := strings.TrimSpace(string(output)) diff --git a/internal/cmd/sdk/generators/go.go b/internal/cmd/sdk/generators/go.go index 0aff5279..b7902900 100644 --- a/internal/cmd/sdk/generators/go.go +++ b/internal/cmd/sdk/generators/go.go @@ -8,6 +8,7 @@ import ( "path/filepath" coreio "github.com/host-uk/core/pkg/io" + "github.com/host-uk/core/pkg/log" ) // GoGenerator generates Go SDKs from OpenAPI specs. @@ -37,7 +38,7 @@ func (g *GoGenerator) Install() string { // Generate creates SDK from OpenAPI spec. func (g *GoGenerator) Generate(ctx context.Context, opts Options) error { if err := coreio.Local.EnsureDir(opts.OutputDir); err != nil { - return fmt.Errorf("go.Generate: failed to create output dir: %w", err) + return log.E("go.Generate", "failed to create output dir", err) } if g.Available() { @@ -59,7 +60,7 @@ func (g *GoGenerator) generateNative(ctx context.Context, opts Options) error { cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { - return fmt.Errorf("go.generateNative: %w", err) + return log.E("go.generateNative", "oapi-codegen failed", err) } goMod := fmt.Sprintf("module %s\n\ngo 1.21\n", opts.PackageName) diff --git a/internal/cmd/setup/cmd_registry.go b/internal/cmd/setup/cmd_registry.go index d9714329..a06e10ef 100644 --- a/internal/cmd/setup/cmd_registry.go +++ b/internal/cmd/setup/cmd_registry.go @@ -117,7 +117,7 @@ func runRegistrySetupWithReg(ctx context.Context, reg *repos.Registry, registryP // Check if already exists repoPath := filepath.Join(basePath, repo.Name) - // Check .git dir existence via List + // Check .git dir existence via Exists if coreio.Local.Exists(filepath.Join(repoPath, ".git")) { exists++ continue diff --git a/pkg/agentic/client.go b/pkg/agentic/client.go index 72675a1a..fe77f937 100644 --- a/pkg/agentic/client.go +++ b/pkg/agentic/client.go @@ -12,7 +12,7 @@ import ( "strings" "time" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // Client is the API client for the core-agentic service. @@ -77,24 +77,24 @@ func (c *Client) ListTasks(ctx context.Context, opts ListOptions) ([]Task, error req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } var tasks []Task if err := json.NewDecoder(resp.Body).Decode(&tasks); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return tasks, nil @@ -105,31 +105,31 @@ func (c *Client) GetTask(ctx context.Context, id string) (*Task, error) { const op = "agentic.Client.GetTask" if id == "" { - return nil, errors.E(op, "task ID is required", nil) + return nil, log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s", c.BaseURL, url.PathEscape(id)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } var task Task if err := json.NewDecoder(resp.Body).Decode(&task); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return &task, nil @@ -140,7 +140,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { const op = "agentic.Client.ClaimTask" if id == "" { - return nil, errors.E(op, "task ID is required", nil) + return nil, log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s/claim", c.BaseURL, url.PathEscape(id)) @@ -154,7 +154,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, body) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -164,18 +164,18 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } // Read body once to allow multiple decode attempts bodyData, err := io.ReadAll(resp.Body) if err != nil { - return nil, errors.E(op, "failed to read response", err) + return nil, log.E(op, "failed to read response", err) } // Try decoding as ClaimResponse first @@ -187,7 +187,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { // Try decoding as just a Task for simpler API responses var task Task if err := json.Unmarshal(bodyData, &task); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return &task, nil @@ -198,19 +198,19 @@ func (c *Client) UpdateTask(ctx context.Context, id string, update TaskUpdate) e const op = "agentic.Client.UpdateTask" if id == "" { - return errors.E(op, "task ID is required", nil) + return log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s", c.BaseURL, url.PathEscape(id)) data, err := json.Marshal(update) if err != nil { - return errors.E(op, "failed to marshal update", err) + return log.E(op, "failed to marshal update", err) } req, err := http.NewRequestWithContext(ctx, http.MethodPatch, endpoint, bytes.NewReader(data)) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -218,12 +218,12 @@ func (c *Client) UpdateTask(ctx context.Context, id string, update TaskUpdate) e resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return errors.E(op, "API error", err) + return log.E(op, "API error", err) } return nil @@ -234,19 +234,19 @@ func (c *Client) CompleteTask(ctx context.Context, id string, result TaskResult) const op = "agentic.Client.CompleteTask" if id == "" { - return errors.E(op, "task ID is required", nil) + return log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s/complete", c.BaseURL, url.PathEscape(id)) data, err := json.Marshal(result) if err != nil { - return errors.E(op, "failed to marshal result", err) + return log.E(op, "failed to marshal result", err) } req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(data)) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -254,12 +254,12 @@ func (c *Client) CompleteTask(ctx context.Context, id string, result TaskResult) resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return errors.E(op, "API error", err) + return log.E(op, "API error", err) } return nil @@ -303,19 +303,19 @@ func (c *Client) Ping(ctx context.Context) error { req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if resp.StatusCode >= 400 { - return errors.E(op, fmt.Sprintf("server returned status %d", resp.StatusCode), nil) + return log.E(op, fmt.Sprintf("server returned status %d", resp.StatusCode), nil) } return nil diff --git a/pkg/agentic/completion.go b/pkg/agentic/completion.go index 3107c876..4a5b58f8 100644 --- a/pkg/agentic/completion.go +++ b/pkg/agentic/completion.go @@ -8,7 +8,7 @@ import ( "os/exec" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // PROptions contains options for creating a pull request. @@ -36,11 +36,11 @@ func AutoCommit(ctx context.Context, task *Task, dir string, message string) err const op = "agentic.AutoCommit" if task == nil { - return errors.E(op, "task is required", nil) + return log.E(op, "task is required", nil) } if message == "" { - return errors.E(op, "commit message is required", nil) + return log.E(op, "commit message is required", nil) } // Build full commit message @@ -48,12 +48,12 @@ func AutoCommit(ctx context.Context, task *Task, dir string, message string) err // Stage all changes if _, err := runGitCommandCtx(ctx, dir, "add", "-A"); err != nil { - return errors.E(op, "failed to stage changes", err) + return log.E(op, "failed to stage changes", err) } // Create commit if _, err := runGitCommandCtx(ctx, dir, "commit", "-m", fullMessage); err != nil { - return errors.E(op, "failed to create commit", err) + return log.E(op, "failed to create commit", err) } return nil @@ -83,7 +83,7 @@ func CreatePR(ctx context.Context, task *Task, dir string, opts PROptions) (stri const op = "agentic.CreatePR" if task == nil { - return "", errors.E(op, "task is required", nil) + return "", log.E(op, "task is required", nil) } // Build title if not provided @@ -116,7 +116,7 @@ func CreatePR(ctx context.Context, task *Task, dir string, opts PROptions) (stri // Run gh pr create output, err := runCommandCtx(ctx, dir, "gh", args...) if err != nil { - return "", errors.E(op, "failed to create PR", err) + return "", log.E(op, "failed to create PR", err) } // Extract PR URL from output @@ -158,11 +158,11 @@ func SyncStatus(ctx context.Context, client *Client, task *Task, update TaskUpda const op = "agentic.SyncStatus" if client == nil { - return errors.E(op, "client is required", nil) + return log.E(op, "client is required", nil) } if task == nil { - return errors.E(op, "task is required", nil) + return log.E(op, "task is required", nil) } return client.UpdateTask(ctx, task.ID, update) @@ -174,7 +174,7 @@ func CommitAndSync(ctx context.Context, client *Client, task *Task, dir string, // Create commit if err := AutoCommit(ctx, task, dir, message); err != nil { - return errors.E(op, "failed to commit", err) + return log.E(op, "failed to commit", err) } // Sync status if client provided @@ -187,7 +187,7 @@ func CommitAndSync(ctx context.Context, client *Client, task *Task, dir string, if err := SyncStatus(ctx, client, task, update); err != nil { // Log but don't fail on sync errors - return errors.E(op, "commit succeeded but sync failed", err) + return log.E(op, "commit succeeded but sync failed", err) } } @@ -200,7 +200,7 @@ func PushChanges(ctx context.Context, dir string) error { _, err := runGitCommandCtx(ctx, dir, "push") if err != nil { - return errors.E(op, "failed to push changes", err) + return log.E(op, "failed to push changes", err) } return nil @@ -211,7 +211,7 @@ func CreateBranch(ctx context.Context, task *Task, dir string) (string, error) { const op = "agentic.CreateBranch" if task == nil { - return "", errors.E(op, "task is required", nil) + return "", log.E(op, "task is required", nil) } // Generate branch name from task @@ -220,7 +220,7 @@ func CreateBranch(ctx context.Context, task *Task, dir string) (string, error) { // Create and checkout branch _, err := runGitCommandCtx(ctx, dir, "checkout", "-b", branchName) if err != nil { - return "", errors.E(op, "failed to create branch", err) + return "", log.E(op, "failed to create branch", err) } return branchName, nil @@ -302,7 +302,7 @@ func GetCurrentBranch(ctx context.Context, dir string) (string, error) { output, err := runGitCommandCtx(ctx, dir, "rev-parse", "--abbrev-ref", "HEAD") if err != nil { - return "", errors.E(op, "failed to get current branch", err) + return "", log.E(op, "failed to get current branch", err) } return strings.TrimSpace(output), nil @@ -314,7 +314,7 @@ func HasUncommittedChanges(ctx context.Context, dir string) (bool, error) { output, err := runGitCommandCtx(ctx, dir, "status", "--porcelain") if err != nil { - return false, errors.E(op, "failed to get git status", err) + return false, log.E(op, "failed to get git status", err) } return strings.TrimSpace(output) != "", nil @@ -331,7 +331,7 @@ func GetDiff(ctx context.Context, dir string, staged bool) (string, error) { output, err := runGitCommandCtx(ctx, dir, args...) if err != nil { - return "", errors.E(op, "failed to get diff", err) + return "", log.E(op, "failed to get diff", err) } return output, nil diff --git a/pkg/agentic/config.go b/pkg/agentic/config.go index 3ad088ad..c713de5f 100644 --- a/pkg/agentic/config.go +++ b/pkg/agentic/config.go @@ -6,7 +6,7 @@ import ( "path/filepath" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" "gopkg.in/yaml.v3" ) @@ -74,12 +74,12 @@ func LoadConfig(dir string) (*Config, error) { // Try loading from ~/.core/agentic.yaml homeDir, err := os.UserHomeDir() if err != nil { - return nil, errors.E("agentic.LoadConfig", "failed to get home directory", err) + return nil, log.E("agentic.LoadConfig", "failed to get home directory", err) } configPath := filepath.Join(homeDir, ".core", configFileName) if err := loadYAMLConfig(configPath, cfg); err != nil && !os.IsNotExist(err) { - return nil, errors.E("agentic.LoadConfig", "failed to load config", err) + return nil, log.E("agentic.LoadConfig", "failed to load config", err) } // Apply environment variable overrides @@ -87,7 +87,7 @@ func LoadConfig(dir string) (*Config, error) { // Validate configuration if cfg.Token == "" { - return nil, errors.E("agentic.LoadConfig", "no authentication token configured", nil) + return nil, log.E("agentic.LoadConfig", "no authentication token configured", nil) } return cfg, nil @@ -167,23 +167,23 @@ func applyEnvOverrides(cfg *Config) { func SaveConfig(cfg *Config) error { homeDir, err := os.UserHomeDir() if err != nil { - return errors.E("agentic.SaveConfig", "failed to get home directory", err) + return log.E("agentic.SaveConfig", "failed to get home directory", err) } configDir := filepath.Join(homeDir, ".core") if err := os.MkdirAll(configDir, 0755); err != nil { - return errors.E("agentic.SaveConfig", "failed to create config directory", err) + return log.E("agentic.SaveConfig", "failed to create config directory", err) } configPath := filepath.Join(configDir, configFileName) data, err := yaml.Marshal(cfg) if err != nil { - return errors.E("agentic.SaveConfig", "failed to marshal config", err) + return log.E("agentic.SaveConfig", "failed to marshal config", err) } if err := os.WriteFile(configPath, data, 0600); err != nil { - return errors.E("agentic.SaveConfig", "failed to write config file", err) + return log.E("agentic.SaveConfig", "failed to write config file", err) } return nil @@ -193,7 +193,7 @@ func SaveConfig(cfg *Config) error { func ConfigPath() (string, error) { homeDir, err := os.UserHomeDir() if err != nil { - return "", errors.E("agentic.ConfigPath", "failed to get home directory", err) + return "", log.E("agentic.ConfigPath", "failed to get home directory", err) } return filepath.Join(homeDir, ".core", configFileName), nil } diff --git a/pkg/agentic/context.go b/pkg/agentic/context.go index a31ba632..00951322 100644 --- a/pkg/agentic/context.go +++ b/pkg/agentic/context.go @@ -9,7 +9,7 @@ import ( "regexp" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // FileContent represents the content of a file for AI context. @@ -41,13 +41,13 @@ func BuildTaskContext(task *Task, dir string) (*TaskContext, error) { const op = "agentic.BuildTaskContext" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } if dir == "" { cwd, err := os.Getwd() if err != nil { - return nil, errors.E(op, "failed to get working directory", err) + return nil, log.E(op, "failed to get working directory", err) } dir = cwd } @@ -87,7 +87,7 @@ func GatherRelatedFiles(task *Task, dir string) ([]FileContent, error) { const op = "agentic.GatherRelatedFiles" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } var files []FileContent @@ -117,7 +117,7 @@ func findRelatedCode(task *Task, dir string) ([]FileContent, error) { const op = "agentic.findRelatedCode" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } // Extract keywords from title and description diff --git a/pkg/container/state.go b/pkg/container/state.go index 53ab1e2a..b8d98b97 100644 --- a/pkg/container/state.go +++ b/pkg/container/state.go @@ -99,13 +99,19 @@ func (s *State) Add(c *Container) error { return s.SaveState() } -// Get retrieves a container by ID. +// Get retrieves a copy of a container by ID. +// Returns a copy to prevent data races when the container is modified. func (s *State) Get(id string) (*Container, bool) { s.mu.RLock() defer s.mu.RUnlock() c, ok := s.Containers[id] - return c, ok + if !ok { + return nil, false + } + // Return a copy to prevent data races + copy := *c + return ©, true } // Update updates a container in the state and persists it. @@ -126,14 +132,16 @@ func (s *State) Remove(id string) error { return s.SaveState() } -// All returns all containers in the state. +// All returns copies of all containers in the state. +// Returns copies to prevent data races when containers are modified. func (s *State) All() []*Container { s.mu.RLock() defer s.mu.RUnlock() containers := make([]*Container, 0, len(s.Containers)) for _, c := range s.Containers { - containers = append(containers, c) + copy := *c + containers = append(containers, ©) } return containers } diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 19741d13..36295762 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -1,151 +1,128 @@ // Package errors provides structured error handling for Core applications. // -// Errors include operational context (what was being done) and support -// error wrapping for debugging while keeping user-facing messages clean: +// Deprecated: Use pkg/log instead. This package is maintained for backward +// compatibility and will be removed in a future version. All error handling +// functions are now available in pkg/log: // -// err := errors.E("user.Create", "email already exists", nil) -// err := errors.Wrap(dbErr, "user.Create", "failed to save user") +// // Instead of: +// import "github.com/host-uk/core/pkg/errors" +// err := errors.E("op", "msg", cause) // -// // Check error types -// if errors.Is(err, sql.ErrNoRows) { ... } +// // Use: +// import "github.com/host-uk/core/pkg/log" +// err := log.E("op", "msg", cause) // -// // Extract operation -// var e *errors.Error -// if errors.As(err, &e) { -// fmt.Println("Operation:", e.Op) -// } +// Migration guide: +// - errors.Error -> log.Err +// - errors.E -> log.E +// - errors.Wrap -> log.Wrap +// - errors.WrapCode -> log.WrapCode +// - errors.Code -> log.NewCode +// - errors.New -> log.NewError +// - errors.Is -> log.Is +// - errors.As -> log.As +// - errors.Join -> log.Join +// - errors.Op -> log.Op +// - errors.ErrCode -> log.ErrCode +// - errors.Message -> log.Message +// - errors.Root -> log.Root package errors import ( - stderrors "errors" - "fmt" + "github.com/host-uk/core/pkg/log" ) // Error represents a structured error with operational context. -type Error struct { - Op string // Operation being performed (e.g., "user.Create") - Msg string // Human-readable message - Err error // Underlying error (optional) - Code string // Error code for i18n/categorisation (optional) -} +// +// Deprecated: Use log.Err instead. +type Error = log.Err // E creates a new Error with operation context. // -// err := errors.E("config.Load", "file not found", os.ErrNotExist) -// err := errors.E("api.Call", "rate limited", nil) +// Deprecated: Use log.E instead. func E(op, msg string, err error) error { - return &Error{Op: op, Msg: msg, Err: err} + return log.E(op, msg, err) } // Wrap wraps an error with operation context. // Returns nil if err is nil. // -// return errors.Wrap(err, "db.Query", "failed to fetch user") +// Deprecated: Use log.Wrap instead. func Wrap(err error, op, msg string) error { - if err == nil { - return nil - } - return &Error{Op: op, Msg: msg, Err: err} + return log.Wrap(err, op, msg) } // WrapCode wraps an error with operation context and an error code. // -// return errors.WrapCode(err, "ERR_NOT_FOUND", "user.Get", "user not found") +// Deprecated: Use log.WrapCode instead. func WrapCode(err error, code, op, msg string) error { - if err == nil && code == "" { - return nil - } - return &Error{Op: op, Msg: msg, Err: err, Code: code} + return log.WrapCode(err, code, op, msg) } // Code creates an error with just a code and message. // -// return errors.Code("ERR_VALIDATION", "invalid email format") +// Deprecated: Use log.NewCode instead. func Code(code, msg string) error { - return &Error{Code: code, Msg: msg} -} - -// Error returns the error message. -func (e *Error) Error() string { - if e.Op != "" && e.Err != nil { - return fmt.Sprintf("%s: %s: %v", e.Op, e.Msg, e.Err) - } - if e.Op != "" { - return fmt.Sprintf("%s: %s", e.Op, e.Msg) - } - if e.Err != nil { - return fmt.Sprintf("%s: %v", e.Msg, e.Err) - } - return e.Msg -} - -// Unwrap returns the underlying error. -func (e *Error) Unwrap() error { - return e.Err + return log.NewCode(code, msg) } // --- Standard library wrappers --- // Is reports whether any error in err's tree matches target. +// +// Deprecated: Use log.Is instead. func Is(err, target error) bool { - return stderrors.Is(err, target) + return log.Is(err, target) } // As finds the first error in err's tree that matches target. +// +// Deprecated: Use log.As instead. func As(err error, target any) bool { - return stderrors.As(err, target) + return log.As(err, target) } // New returns an error with the given text. +// +// Deprecated: Use log.NewError instead. func New(text string) error { - return stderrors.New(text) + return log.NewError(text) } // Join returns an error that wraps the given errors. +// +// Deprecated: Use log.Join instead. func Join(errs ...error) error { - return stderrors.Join(errs...) + return log.Join(errs...) } // --- Helper functions --- // Op extracts the operation from an error, or empty string if not an Error. +// +// Deprecated: Use log.Op instead. func Op(err error) string { - var e *Error - if As(err, &e) { - return e.Op - } - return "" + return log.Op(err) } // ErrCode extracts the error code, or empty string if not set. +// +// Deprecated: Use log.ErrCode instead. func ErrCode(err error) string { - var e *Error - if As(err, &e) { - return e.Code - } - return "" + return log.ErrCode(err) } // Message extracts the message from an error. // For Error types, returns Msg; otherwise returns err.Error(). +// +// Deprecated: Use log.Message instead. func Message(err error) string { - if err == nil { - return "" - } - var e *Error - if As(err, &e) { - return e.Msg - } - return err.Error() + return log.Message(err) } // Root returns the deepest error in the chain. +// +// Deprecated: Use log.Root instead. func Root(err error) error { - for { - unwrapped := stderrors.Unwrap(err) - if unwrapped == nil { - return err - } - err = unwrapped - } + return log.Root(err) } diff --git a/pkg/io/local/client.go b/pkg/io/local/client.go index f17a4da5..4e232fef 100644 --- a/pkg/io/local/client.go +++ b/pkg/io/local/client.go @@ -31,6 +31,10 @@ func (m *Medium) path(p string) string { } clean := strings.ReplaceAll(p, "..", ".") if filepath.IsAbs(clean) { + // Handle Windows drive root (e.g. "C:\") + if len(clean) == 3 && clean[1] == ':' && (clean[2] == '\\' || clean[2] == '/') { + return clean + } return filepath.Clean(clean) } return filepath.Join(m.root, clean) diff --git a/pkg/log/errors.go b/pkg/log/errors.go new file mode 100644 index 00000000..c6775521 --- /dev/null +++ b/pkg/log/errors.go @@ -0,0 +1,229 @@ +// Package log provides structured logging and error handling for Core applications. +// +// This file implements structured error types and combined log-and-return helpers +// that simplify common error handling patterns. + +package log + +import ( + "errors" + "fmt" +) + +// Err represents a structured error with operational context. +// It implements the error interface and supports unwrapping. +type Err struct { + Op string // Operation being performed (e.g., "user.Save") + Msg string // Human-readable message + Err error // Underlying error (optional) + Code string // Error code (optional, e.g., "VALIDATION_FAILED") +} + +// Error implements the error interface. +func (e *Err) Error() string { + var prefix string + if e.Op != "" { + prefix = e.Op + ": " + } + if e.Err != nil { + if e.Code != "" { + return fmt.Sprintf("%s%s [%s]: %v", prefix, e.Msg, e.Code, e.Err) + } + return fmt.Sprintf("%s%s: %v", prefix, e.Msg, e.Err) + } + if e.Code != "" { + return fmt.Sprintf("%s%s [%s]", prefix, e.Msg, e.Code) + } + return fmt.Sprintf("%s%s", prefix, e.Msg) +} + +// Unwrap returns the underlying error for use with errors.Is and errors.As. +func (e *Err) Unwrap() error { + return e.Err +} + +// --- Error Creation Functions --- + +// E creates a new Err with operation context. +// The underlying error can be nil for creating errors without a cause. +// +// Example: +// +// return log.E("user.Save", "failed to save user", err) +// return log.E("api.Call", "rate limited", nil) // No underlying cause +func E(op, msg string, err error) error { + return &Err{Op: op, Msg: msg, Err: err} +} + +// Wrap wraps an error with operation context. +// Returns nil if err is nil, to support conditional wrapping. +// Preserves error Code if the wrapped error is an *Err. +// +// Example: +// +// return log.Wrap(err, "db.Query", "database query failed") +func Wrap(err error, op, msg string) error { + if err == nil { + return nil + } + // Preserve Code from wrapped *Err + var logErr *Err + if As(err, &logErr) && logErr.Code != "" { + return &Err{Op: op, Msg: msg, Err: err, Code: logErr.Code} + } + return &Err{Op: op, Msg: msg, Err: err} +} + +// WrapCode wraps an error with operation context and error code. +// Returns nil only if both err is nil AND code is empty. +// Useful for API errors that need machine-readable codes. +// +// Example: +// +// return log.WrapCode(err, "VALIDATION_ERROR", "user.Validate", "invalid email") +func WrapCode(err error, code, op, msg string) error { + if err == nil && code == "" { + return nil + } + return &Err{Op: op, Msg: msg, Err: err, Code: code} +} + +// NewCode creates an error with just code and message (no underlying error). +// Useful for creating sentinel errors with codes. +// +// Example: +// +// var ErrNotFound = log.NewCode("NOT_FOUND", "resource not found") +func NewCode(code, msg string) error { + return &Err{Msg: msg, Code: code} +} + +// --- Standard Library Wrappers --- + +// Is reports whether any error in err's tree matches target. +// Wrapper around errors.Is for convenience. +func Is(err, target error) bool { + return errors.Is(err, target) +} + +// As finds the first error in err's tree that matches target. +// Wrapper around errors.As for convenience. +func As(err error, target any) bool { + return errors.As(err, target) +} + +// NewError creates a simple error with the given text. +// Wrapper around errors.New for convenience. +func NewError(text string) error { + return errors.New(text) +} + +// Join combines multiple errors into one. +// Wrapper around errors.Join for convenience. +func Join(errs ...error) error { + return errors.Join(errs...) +} + +// --- Error Introspection Helpers --- + +// Op extracts the operation name from an error. +// Returns empty string if the error is not an *Err. +func Op(err error) string { + var e *Err + if As(err, &e) { + return e.Op + } + return "" +} + +// ErrCode extracts the error code from an error. +// Returns empty string if the error is not an *Err or has no code. +func ErrCode(err error) string { + var e *Err + if As(err, &e) { + return e.Code + } + return "" +} + +// Message extracts the message from an error. +// Returns the error's Error() string if not an *Err. +func Message(err error) string { + if err == nil { + return "" + } + var e *Err + if As(err, &e) { + return e.Msg + } + return err.Error() +} + +// Root returns the root cause of an error chain. +// Unwraps until no more wrapped errors are found. +func Root(err error) error { + if err == nil { + return nil + } + for { + unwrapped := errors.Unwrap(err) + if unwrapped == nil { + return err + } + err = unwrapped + } +} + +// --- Combined Log-and-Return Helpers --- + +// LogError logs an error at Error level and returns a wrapped error. +// Reduces boilerplate in error handling paths. +// +// Example: +// +// // Before +// if err != nil { +// log.Error("failed to save", "err", err) +// return errors.Wrap(err, "user.Save", "failed to save") +// } +// +// // After +// if err != nil { +// return log.LogError(err, "user.Save", "failed to save") +// } +func LogError(err error, op, msg string) error { + if err == nil { + return nil + } + wrapped := Wrap(err, op, msg) + defaultLogger.Error(msg, "op", op, "err", err) + return wrapped +} + +// LogWarn logs at Warn level and returns a wrapped error. +// Use for recoverable errors that should be logged but not treated as critical. +// +// Example: +// +// return log.LogWarn(err, "cache.Get", "cache miss, falling back to db") +func LogWarn(err error, op, msg string) error { + if err == nil { + return nil + } + wrapped := Wrap(err, op, msg) + defaultLogger.Warn(msg, "op", op, "err", err) + return wrapped +} + +// Must panics if err is not nil, logging first. +// Use for errors that should never happen and indicate programmer error. +// +// Example: +// +// log.Must(Initialize(), "app", "startup failed") +func Must(err error, op, msg string) { + if err != nil { + defaultLogger.Error(msg, "op", op, "err", err) + panic(Wrap(err, op, msg)) + } +} diff --git a/pkg/log/errors_test.go b/pkg/log/errors_test.go new file mode 100644 index 00000000..96cbd12f --- /dev/null +++ b/pkg/log/errors_test.go @@ -0,0 +1,305 @@ +package log + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// --- Err Type Tests --- + +func TestErr_Error_Good(t *testing.T) { + // With underlying error + err := &Err{Op: "db.Query", Msg: "failed to query", Err: errors.New("connection refused")} + assert.Equal(t, "db.Query: failed to query: connection refused", err.Error()) + + // With code + err = &Err{Op: "api.Call", Msg: "request failed", Code: "TIMEOUT"} + assert.Equal(t, "api.Call: request failed [TIMEOUT]", err.Error()) + + // With both underlying error and code + err = &Err{Op: "user.Save", Msg: "save failed", Err: errors.New("duplicate key"), Code: "DUPLICATE"} + assert.Equal(t, "user.Save: save failed [DUPLICATE]: duplicate key", err.Error()) + + // Just op and msg + err = &Err{Op: "cache.Get", Msg: "miss"} + assert.Equal(t, "cache.Get: miss", err.Error()) +} + +func TestErr_Error_EmptyOp_Good(t *testing.T) { + // No Op - should not have leading colon + err := &Err{Msg: "just a message"} + assert.Equal(t, "just a message", err.Error()) + + // No Op with code + err = &Err{Msg: "error with code", Code: "ERR_CODE"} + assert.Equal(t, "error with code [ERR_CODE]", err.Error()) + + // No Op with underlying error + err = &Err{Msg: "wrapped", Err: errors.New("underlying")} + assert.Equal(t, "wrapped: underlying", err.Error()) +} + +func TestErr_Unwrap_Good(t *testing.T) { + underlying := errors.New("underlying error") + err := &Err{Op: "test", Msg: "wrapped", Err: underlying} + + assert.Equal(t, underlying, errors.Unwrap(err)) + assert.True(t, errors.Is(err, underlying)) +} + +// --- Error Creation Function Tests --- + +func TestE_Good(t *testing.T) { + underlying := errors.New("base error") + err := E("op.Name", "something failed", underlying) + + assert.NotNil(t, err) + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "op.Name", logErr.Op) + assert.Equal(t, "something failed", logErr.Msg) + assert.Equal(t, underlying, logErr.Err) +} + +func TestE_Good_NilError(t *testing.T) { + // E creates an error even with nil underlying - useful for errors without causes + err := E("op.Name", "message", nil) + assert.NotNil(t, err) + assert.Equal(t, "op.Name: message", err.Error()) +} + +func TestWrap_Good(t *testing.T) { + underlying := errors.New("base") + err := Wrap(underlying, "handler.Process", "processing failed") + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "handler.Process") + assert.Contains(t, err.Error(), "processing failed") + assert.True(t, errors.Is(err, underlying)) +} + +func TestWrap_PreservesCode_Good(t *testing.T) { + // Create an error with a code + inner := WrapCode(errors.New("base"), "VALIDATION_ERROR", "inner.Op", "validation failed") + + // Wrap it - should preserve the code + outer := Wrap(inner, "outer.Op", "outer context") + + assert.NotNil(t, outer) + assert.Equal(t, "VALIDATION_ERROR", ErrCode(outer)) + assert.Contains(t, outer.Error(), "[VALIDATION_ERROR]") +} + +func TestWrap_NilError_Good(t *testing.T) { + err := Wrap(nil, "op", "msg") + assert.Nil(t, err) +} + +func TestWrapCode_Good(t *testing.T) { + underlying := errors.New("validation failed") + err := WrapCode(underlying, "INVALID_INPUT", "api.Validate", "bad request") + + assert.NotNil(t, err) + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "INVALID_INPUT", logErr.Code) + assert.Equal(t, "api.Validate", logErr.Op) + assert.Contains(t, err.Error(), "[INVALID_INPUT]") +} + +func TestWrapCode_Good_NilError(t *testing.T) { + // WrapCode with nil error but with code still creates an error + err := WrapCode(nil, "CODE", "op", "msg") + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "[CODE]") + + // Only returns nil when both error and code are empty + err = WrapCode(nil, "", "op", "msg") + assert.Nil(t, err) +} + +func TestNewCode_Good(t *testing.T) { + err := NewCode("NOT_FOUND", "resource not found") + + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "NOT_FOUND", logErr.Code) + assert.Equal(t, "resource not found", logErr.Msg) + assert.Nil(t, logErr.Err) +} + +// --- Standard Library Wrapper Tests --- + +func TestIs_Good(t *testing.T) { + sentinel := errors.New("sentinel") + wrapped := Wrap(sentinel, "test", "wrapped") + + assert.True(t, Is(wrapped, sentinel)) + assert.False(t, Is(wrapped, errors.New("other"))) +} + +func TestAs_Good(t *testing.T) { + err := E("test.Op", "message", errors.New("base")) + + var logErr *Err + assert.True(t, As(err, &logErr)) + assert.Equal(t, "test.Op", logErr.Op) +} + +func TestNewError_Good(t *testing.T) { + err := NewError("simple error") + assert.NotNil(t, err) + assert.Equal(t, "simple error", err.Error()) +} + +func TestJoin_Good(t *testing.T) { + err1 := errors.New("error 1") + err2 := errors.New("error 2") + joined := Join(err1, err2) + + assert.True(t, errors.Is(joined, err1)) + assert.True(t, errors.Is(joined, err2)) +} + +// --- Helper Function Tests --- + +func TestOp_Good(t *testing.T) { + err := E("mypackage.MyFunc", "failed", errors.New("cause")) + assert.Equal(t, "mypackage.MyFunc", Op(err)) +} + +func TestOp_Good_NotLogError(t *testing.T) { + err := errors.New("plain error") + assert.Equal(t, "", Op(err)) +} + +func TestErrCode_Good(t *testing.T) { + err := WrapCode(errors.New("base"), "ERR_CODE", "op", "msg") + assert.Equal(t, "ERR_CODE", ErrCode(err)) +} + +func TestErrCode_Good_NoCode(t *testing.T) { + err := E("op", "msg", errors.New("base")) + assert.Equal(t, "", ErrCode(err)) +} + +func TestMessage_Good(t *testing.T) { + err := E("op", "the message", errors.New("base")) + assert.Equal(t, "the message", Message(err)) +} + +func TestMessage_Good_PlainError(t *testing.T) { + err := errors.New("plain message") + assert.Equal(t, "plain message", Message(err)) +} + +func TestMessage_Good_Nil(t *testing.T) { + assert.Equal(t, "", Message(nil)) +} + +func TestRoot_Good(t *testing.T) { + root := errors.New("root cause") + level1 := Wrap(root, "level1", "wrapped once") + level2 := Wrap(level1, "level2", "wrapped twice") + + assert.Equal(t, root, Root(level2)) +} + +func TestRoot_Good_SingleError(t *testing.T) { + err := errors.New("single") + assert.Equal(t, err, Root(err)) +} + +func TestRoot_Good_Nil(t *testing.T) { + assert.Nil(t, Root(nil)) +} + +// --- Log-and-Return Helper Tests --- + +func TestLogError_Good(t *testing.T) { + // Capture log output + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + underlying := errors.New("connection failed") + err := LogError(underlying, "db.Connect", "database unavailable") + + // Check returned error + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "db.Connect") + assert.Contains(t, err.Error(), "database unavailable") + assert.True(t, errors.Is(err, underlying)) + + // Check log output + output := buf.String() + assert.Contains(t, output, "[ERR]") + assert.Contains(t, output, "database unavailable") + assert.Contains(t, output, "op=db.Connect") +} + +func TestLogError_Good_NilError(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + err := LogError(nil, "op", "msg") + assert.Nil(t, err) + assert.Empty(t, buf.String()) // No log output for nil error +} + +func TestLogWarn_Good(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + underlying := errors.New("cache miss") + err := LogWarn(underlying, "cache.Get", "falling back to db") + + assert.NotNil(t, err) + assert.True(t, errors.Is(err, underlying)) + + output := buf.String() + assert.Contains(t, output, "[WRN]") + assert.Contains(t, output, "falling back to db") +} + +func TestLogWarn_Good_NilError(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + err := LogWarn(nil, "op", "msg") + assert.Nil(t, err) + assert.Empty(t, buf.String()) +} + +func TestMust_Good_NoError(t *testing.T) { + // Should not panic when error is nil + assert.NotPanics(t, func() { + Must(nil, "test", "should not panic") + }) +} + +func TestMust_Ugly_Panics(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + assert.Panics(t, func() { + Must(errors.New("fatal error"), "startup", "initialization failed") + }) + + // Verify error was logged before panic + output := buf.String() + assert.True(t, strings.Contains(output, "[ERR]") || len(output) > 0) +} diff --git a/pkg/mcp/transport_tcp.go b/pkg/mcp/transport_tcp.go index f7b5f1e5..0e6e0f7e 100644 --- a/pkg/mcp/transport_tcp.go +++ b/pkg/mcp/transport_tcp.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "fmt" + "io" "net" "os" @@ -11,6 +12,9 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) +// maxMCPMessageSize is the maximum size for MCP JSON-RPC messages (10 MB). +const maxMCPMessageSize = 10 * 1024 * 1024 + // TCPTransport manages a TCP listener for MCP. type TCPTransport struct { addr string @@ -36,6 +40,12 @@ func (s *Service) ServeTCP(ctx context.Context, addr string) error { } defer t.listener.Close() + // Close listener when context is cancelled to unblock Accept + go func() { + <-ctx.Done() + t.listener.Close() + }() + if addr == "" { addr = t.listener.Addr().String() } @@ -84,9 +94,11 @@ type connTransport struct { } func (t *connTransport) Connect(ctx context.Context) (mcp.Connection, error) { + scanner := bufio.NewScanner(t.conn) + scanner.Buffer(make([]byte, 64*1024), maxMCPMessageSize) return &connConnection{ conn: t.conn, - scanner: bufio.NewScanner(t.conn), + scanner: scanner, }, nil } @@ -102,10 +114,8 @@ func (c *connConnection) Read(ctx context.Context) (jsonrpc.Message, error) { if err := c.scanner.Err(); err != nil { return nil, err } - // EOF - // Return error to signal closure, as per Scanner contract? - // SDK usually expects error on close. - return nil, fmt.Errorf("EOF") + // EOF - connection closed cleanly + return nil, io.EOF } line := c.scanner.Bytes() return jsonrpc.DecodeMessage(line)