feat(log): Logging enhancements (#181)
* feat(help): Add CLI help command Fixes #136 * chore: remove binary * feat(mcp): Add TCP transport Fixes #126 * feat(io): Migrate pkg/mcp to use Medium abstraction Fixes #103 * feat(io): batch implementation placeholder Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(log): batch implementation placeholder Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(io): Migrate internal/cmd/docs/* to Medium abstraction Fixes #113 * chore(io): Migrate internal/cmd/dev/* to Medium abstraction Fixes #114 * chore(io): Migrate internal/cmd/setup/* to Medium abstraction * chore(io): Complete migration of internal/cmd/dev/* to Medium abstraction * feat(io): extend Medium interface with Delete, Rename, List, Stat operations Adds the following methods to the Medium interface: - Delete(path) - remove a file or empty directory - DeleteAll(path) - recursively remove a file or directory - Rename(old, new) - move/rename a file or directory - List(path) - list directory entries (returns []fs.DirEntry) - Stat(path) - get file information (returns fs.FileInfo) - Exists(path) - check if path exists - IsDir(path) - check if path is a directory Implements these methods in both local.Medium (using os package) and MockMedium (in-memory for testing). Includes FileInfo and DirEntry types for mock implementations. This enables migration of direct os.* calls to the Medium abstraction for consistent path validation and testability. Refs #101 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(io): Migrate internal/cmd/sdk, pkgcmd, and workspace to Medium abstraction * chore(io): migrate internal/cmd/docs and internal/cmd/dev to Medium - internal/cmd/docs: Replace os.Stat, os.ReadFile, os.WriteFile, os.MkdirAll, os.RemoveAll with io.Local equivalents - internal/cmd/dev: Replace os.Stat, os.ReadFile, os.WriteFile, os.MkdirAll, os.ReadDir with io.Local equivalents - Fix local.Medium to allow absolute paths when root is "/" for full filesystem access (io.Local use case) Refs #113, #114 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(io): migrate internal/cmd/setup to Medium abstraction Migrated all direct os.* filesystem calls to use io.Local: - cmd_repo.go: os.MkdirAll -> io.Local.EnsureDir, os.WriteFile -> io.Local.Write, os.Stat -> io.Local.IsFile - cmd_bootstrap.go: os.MkdirAll -> io.Local.EnsureDir, os.Stat -> io.Local.IsDir/Exists, os.ReadDir -> io.Local.List - cmd_registry.go: os.MkdirAll -> io.Local.EnsureDir, os.Stat -> io.Local.Exists - cmd_ci.go: os.ReadFile -> io.Local.Read - github_config.go: os.ReadFile -> io.Local.Read, os.Stat -> io.Local.Exists Refs #116 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(log): add error creation and log-and-return helpers Implements issues #129 and #132: - Add Err struct with Op, Msg, Err, Code fields for structured errors - Add E(), Wrap(), WrapCode(), NewCode() for error creation - Add Is(), As(), NewError(), Join() as stdlib wrappers - Add Op(), ErrCode(), Message(), Root() for introspection - Add LogError(), LogWarn(), Must() for combined log-and-return Closes #129 Closes #132 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(io): address Copilot review feedback - Fix MockMedium.Rename: collect keys before mutating maps during iteration - Fix .git checks to use Exists instead of List (handles worktrees/submodules) - Fix cmd_sync.go: use DeleteAll for recursive directory removal Files updated: - pkg/io/io.go: safe map iteration in Rename - internal/cmd/setup/cmd_bootstrap.go: Exists for .git checks - internal/cmd/setup/cmd_registry.go: Exists for .git checks - internal/cmd/pkgcmd/cmd_install.go: Exists for .git checks - internal/cmd/pkgcmd/cmd_manage.go: Exists for .git checks - internal/cmd/docs/cmd_sync.go: DeleteAll for recursive delete Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(updater): resolve PkgVersion duplicate declaration Remove var PkgVersion from updater.go since go generate creates const PkgVersion in version.go. Track version.go in git to ensure builds work without running go generate first. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * style: fix formatting in internal/variants Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(io): simplify local Medium implementation Rewrote to match the simpler TypeScript pattern: - path() sanitizes and returns string directly - Each method calls path() once - No complex symlink validation - Less code, less attack surface Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(io): remove duplicate method declarations Clean up the client.go file that had duplicate method declarations from a bad cherry-pick merge. Now has 127 lines of simple, clean code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(io): fix traversal test to match sanitization behavior The simplified path() sanitizes .. to . without returning errors. Update test to verify sanitization works correctly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(mcp): update sandboxing tests for simplified Medium The simplified io/local.Medium implementation: - Sanitizes .. to . (no error, path is cleaned) - Allows absolute paths through (caller validates if needed) - Follows symlinks (no traversal blocking) Update tests to match this simplified behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * 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> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
4014fd2dc3
commit
89a789d170
7 changed files with 77 additions and 14 deletions
|
|
@ -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++
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -53,6 +57,7 @@ func E(op, msg string, err error) error {
|
|||
|
||||
// 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:
|
||||
//
|
||||
|
|
@ -61,7 +66,12 @@ func Wrap(err error, op, msg string) error {
|
|||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
return E(op, msg, err)
|
||||
// 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.
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
@ -68,6 +82,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")
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue