fix: address CodeRabbit review feedback for PR #181

- internal/cmd/dev/cmd_file_sync.go: Add EnsureDir error handling before Copy
- internal/cmd/docs/cmd_sync.go: Add EnsureDir error handling for parent dirs
- internal/cmd/sdk/generators/go.go: Use log.E() helper instead of fmt.Errorf
- pkg/io/local/client.go: Handle Windows drive-root paths in path()
- pkg/log/errors.go: Avoid leading colon when Op is empty, preserve Code in Wrap
- pkg/log/errors_test.go: Rename tests to follow _Good/_Bad/_Ugly suffix pattern
- pkg/mcp/transport_tcp.go: Fix ctx cancellation, increase scanner buffer, use io.EOF

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

internal/cmd/dev/cmd_file_sync.go

@@ -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++

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)

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)

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)

pkg/log/errors.go

@@ -21,16 +21,20 @@ type Err struct {
 
 // 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", e.Op, e.Msg, e.Code, e.Err)
+			return fmt.Sprintf("%s%s [%s]: %v", prefix, e.Msg, e.Code, e.Err)
 		}
-		return fmt.Sprintf("%s: %s: %v", e.Op, e.Msg, e.Err)
+		return fmt.Sprintf("%s%s: %v", prefix, e.Msg, e.Err)
 	}
 	if e.Code != "" {
-		return fmt.Sprintf("%s: %s [%s]", e.Op, e.Msg, e.Code)
+		return fmt.Sprintf("%s%s [%s]", prefix, e.Msg, e.Code)
 	}
-	return fmt.Sprintf("%s: %s", e.Op, e.Msg)
+	return fmt.Sprintf("%s%s", prefix, e.Msg)
 }
 
 // Unwrap returns the underlying error for use with errors.Is and errors.As.
@@ -54,13 +58,21 @@ func E(op, msg string, err error) error {
 }
 
 // Wrap wraps an error with operation context.
-// Alias for E() for semantic clarity when wrapping existing errors.
+// 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 {
-	return E(op, msg, err)
+	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.

pkg/log/errors_test.go

@@ -29,6 +29,20 @@ func TestErr_Error_Good(t *testing.T) {
 	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}
@@ -51,7 +65,7 @@ func TestE_Good(t *testing.T) {
 	assert.Equal(t, underlying, logErr.Err)
 }
 
-func TestE_Good_NilError(t *testing.T) {
+func TestE_NilError_Good(t *testing.T) {
 	// Should return nil when wrapping nil
 	err := E("op.Name", "message", nil)
 	assert.Nil(t, err)
@@ -67,6 +81,23 @@ func TestWrap_Good(t *testing.T) {
 	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")
@@ -79,7 +110,7 @@ func TestWrapCode_Good(t *testing.T) {
 	assert.Contains(t, err.Error(), "[INVALID_INPUT]")
 }
 
-func TestWrapCode_Good_NilError(t *testing.T) {
+func TestWrapCode_NilError_Good(t *testing.T) {
 	err := WrapCode(nil, "CODE", "op", "msg")
 	assert.Nil(t, err)
 }
@@ -134,7 +165,7 @@ func TestOp_Good(t *testing.T) {
 	assert.Equal(t, "mypackage.MyFunc", Op(err))
 }
 
-func TestOp_Good_NotLogError(t *testing.T) {
+func TestOp_PlainError_Good(t *testing.T) {
 	err := errors.New("plain error")
 	assert.Equal(t, "", Op(err))
 }
@@ -144,7 +175,7 @@ func TestErrCode_Good(t *testing.T) {
 	assert.Equal(t, "ERR_CODE", ErrCode(err))
 }
 
-func TestErrCode_Good_NoCode(t *testing.T) {
+func TestErrCode_NoCode_Good(t *testing.T) {
 	err := E("op", "msg", errors.New("base"))
 	assert.Equal(t, "", ErrCode(err))
 }
@@ -154,12 +185,12 @@ func TestMessage_Good(t *testing.T) {
 	assert.Equal(t, "the message", Message(err))
 }
 
-func TestMessage_Good_PlainError(t *testing.T) {
+func TestMessage_PlainError_Good(t *testing.T) {
 	err := errors.New("plain message")
 	assert.Equal(t, "plain message", Message(err))
 }
 
-func TestMessage_Good_Nil(t *testing.T) {
+func TestMessage_Nil_Good(t *testing.T) {
 	assert.Equal(t, "", Message(nil))
 }
 
@@ -171,12 +202,12 @@ func TestRoot_Good(t *testing.T) {
 	assert.Equal(t, root, Root(level2))
 }
 
-func TestRoot_Good_SingleError(t *testing.T) {
+func TestRoot_SingleError_Good(t *testing.T) {
 	err := errors.New("single")
 	assert.Equal(t, err, Root(err))
 }
 
-func TestRoot_Good_Nil(t *testing.T) {
+func TestRoot_Nil_Good(t *testing.T) {
 	assert.Nil(t, Root(nil))
 }
 
@@ -205,7 +236,7 @@ func TestLogError_Good(t *testing.T) {
 	assert.Contains(t, output, "op=db.Connect")
 }
 
-func TestLogError_Good_NilError(t *testing.T) {
+func TestLogError_NilError_Good(t *testing.T) {
 	var buf bytes.Buffer
 	logger := New(Options{Level: LevelDebug, Output: &buf})
 	SetDefault(logger)
@@ -233,7 +264,7 @@ func TestLogWarn_Good(t *testing.T) {
 	assert.Contains(t, output, "falling back to db")
 }
 
-func TestLogWarn_Good_NilError(t *testing.T) {
+func TestLogWarn_NilError_Good(t *testing.T) {
 	var buf bytes.Buffer
 	logger := New(Options{Level: LevelDebug, Output: &buf})
 	SetDefault(logger)
@@ -244,7 +275,7 @@ func TestLogWarn_Good_NilError(t *testing.T) {
 	assert.Empty(t, buf.String())
 }
 
-func TestMust_Good_NoError(t *testing.T) {
+func TestMust_NoError_Good(t *testing.T) {
 	// Should not panic when error is nil
 	assert.NotPanics(t, func() {
 		Must(nil, "test", "should not panic")

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)