Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ coverage.html
# OS
.DS_Store
Thumbs.db
.core/
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Two files:

## Test Conventions

- `_Good`, `_Bad` suffix pattern for success/failure cases.
- `_Good` / `_Bad` suffix pattern for success / failure cases.
- Tests use real git repos created by `initTestRepo()` in temp directories.
- Service helper tests (in `service_test.go`) construct `Service` structs directly without the framework.
- Framework integration tests (in `service_extra_test.go`) use `core.New()` and test handler dispatch.
Expand Down
18 changes: 13 additions & 5 deletions git.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import (
"bytes"
"context"
"fmt"
"io"
goio "io"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's a naming conflict requiring the io alias
rg -n '\bio\b' git.go | head -20

Repository: dAppCore/go-git

Length of output: 71


🏁 Script executed:

# View the imports section of git.go to see the full context
head -30 git.go

Repository: dAppCore/go-git

Length of output: 551


🏁 Script executed:

# Check if there are any local definitions or variables named 'io' that would conflict
rg -i 'var\s+io\b|type\s+io\b|const\s+io\b|\bio\s*:=' git.go

Repository: dAppCore/go-git

Length of output: 41


🏁 Script executed:

# Check the actual usage at line 205 where goio is reportedly used
sed -n '200,210p' git.go

Repository: dAppCore/go-git

Length of output: 331


🏁 Script executed:

# Search for any custom 'io' package or type definitions
rg -i 'package\s+io\b|type\s+io\b|const\s+io\b' --type-list

Repository: dAppCore/go-git

Length of output: 448


🏁 Script executed:

# Check if other files use the goio alias
rg 'goio\s+"io"' --type go

Repository: dAppCore/go-git

Length of output: 76


🏁 Script executed:

# Look for any reference to local 'io' that might conflict
fd --type f --extension go | xargs grep -l '\bio\s*\.' | head -10

Repository: dAppCore/go-git

Length of output: 41


🏁 Script executed:

# Search for custom io package or type definitions
find . -name "*.go" -type f | xargs grep -l 'type io\b\|package io\b' | grep -v vendor

Repository: dAppCore/go-git

Length of output: 41


🏁 Script executed:

# Check if there are any comments or context explaining why io alias is needed
rg -B5 'goio\s+"io"' git.go

Repository: dAppCore/go-git

Length of output: 106


🏁 Script executed:

# List all Go files to understand repository structure
git ls-files '*.go' | head -20

Repository: dAppCore/go-git

Length of output: 126


🏁 Script executed:

# Check service.go to see if there's a pattern with aliases
head -30 service.go

Repository: dAppCore/go-git

Length of output: 569


🏁 Script executed:

# Search for any 'io' usage in service.go that might explain the pattern
rg -n '\bio\.' service.go | head -10

Repository: dAppCore/go-git

Length of output: 41


Remove the unnecessary io alias.

The goio alias for the standard io package is not required. There is no naming conflict in the file or codebase that would justify it. Replace goio.MultiWriter with io.MultiWriter at line 205 and remove the alias import.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@git.go` at line 8, Remove the unnecessary alias import "goio" and update
references accordingly: in the import block replace the aliased import with the
standard "io" package, and change any usage of goio.MultiWriter to
io.MultiWriter (the occurrence in the function that constructs the writer).
Ensure no other references to goio remain and run gofmt/go vet to verify imports
are clean.

"os"
"os/exec"
"path/filepath"
"slices"
"strconv"
"strings"
"sync"

coreerr "forge.lthn.ai/core/go-log"
)

// RepoStatus represents the git status of a single repository.
Expand Down Expand Up @@ -81,7 +83,7 @@ func getStatus(ctx context.Context, path, name string) RepoStatus {

// Validate path to prevent directory traversal
if !filepath.IsAbs(path) {
status.Error = fmt.Errorf("path must be absolute: %s", path)
status.Error = coreerr.E("git.getStatus", "path must be absolute: "+path, nil)
return status
}

Expand Down Expand Up @@ -200,7 +202,7 @@ func gitInteractive(ctx context.Context, dir string, args ...string) error {

// Capture stderr for error reporting while also showing it
var stderr bytes.Buffer
cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
cmd.Stderr = goio.MultiWriter(os.Stderr, &stderr)

if err := cmd.Run(); err != nil {
return &GitError{
Expand Down Expand Up @@ -272,6 +274,9 @@ func gitCommand(ctx context.Context, dir string, args ...string) (string, error)
return stdout.String(), nil
}

// Compile-time interface checks.
var _ error = (*GitError)(nil)

// GitError wraps a git command error with stderr output and command context.
type GitError struct {
Args []string
Expand All @@ -285,9 +290,12 @@ func (e *GitError) Error() string {
stderr := strings.TrimSpace(e.Stderr)

if stderr != "" {
return fmt.Errorf("git command %q failed: %s", cmd, stderr).Error()
return fmt.Sprintf("git command %q failed: %s", cmd, stderr)
}
if e.Err != nil {
return fmt.Sprintf("git command %q failed: %v", cmd, e.Err)
}
return fmt.Errorf("git command %q failed: %w", cmd, e.Err).Error()
return fmt.Sprintf("git command %q failed", cmd)
}

// Unwrap returns the underlying error for error chain inspection.
Expand Down
7 changes: 7 additions & 0 deletions git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,13 @@ func TestGetStatus_Bad_InvalidPath(t *testing.T) {
assert.Equal(t, "bad-repo", status.Name)
}

func TestGetStatus_Bad_RelativePath(t *testing.T) {
status := getStatus(context.Background(), "relative/path", "rel-repo")
assert.Error(t, status.Error)
assert.Contains(t, status.Error.Error(), "path must be absolute")
assert.Equal(t, "rel-repo", status.Name)
}

// --- Status (parallel multi-repo) tests ---

func TestStatus_Good_MultipleRepos(t *testing.T) {
Expand Down
7 changes: 3 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ module forge.lthn.ai/core/go-git
go 1.26.0

require (
forge.lthn.ai/core/go v0.3.0
forge.lthn.ai/core/go v0.3.1
forge.lthn.ai/core/go-log v0.0.4
github.com/stretchr/testify v1.11.1
)

require (
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/rogpeppe/go-internal v1.14.1 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
11 changes: 4 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
forge.lthn.ai/core/go v0.3.0 h1:mOG97ApMprwx9Ked62FdWVwXTGSF6JO6m0DrVpoH2Q4=
forge.lthn.ai/core/go v0.3.0/go.mod h1:gE6c8h+PJ2287qNhVUJ5SOe1kopEwHEquvinstpuyJc=
forge.lthn.ai/core/go v0.3.1 h1:5FMTsUhLcxSr07F9q3uG0Goy4zq4eLivoqi8shSY4UM=
forge.lthn.ai/core/go v0.3.1/go.mod h1:gE6c8h+PJ2287qNhVUJ5SOe1kopEwHEquvinstpuyJc=
forge.lthn.ai/core/go-log v0.0.4 h1:KTuCEPgFmuM8KJfnyQ8vPOU1Jg654W74h8IJvfQMfv0=
forge.lthn.ai/core/go-log v0.0.4/go.mod h1:r14MXKOD3LF/sI8XUJQhRk/SZHBE7jAFVuCfgkXoZPw=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
Expand Down
9 changes: 6 additions & 3 deletions service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ package git

import (
"context"
"fmt"
"iter"
"path/filepath"
"slices"
"strings"
"sync"

"forge.lthn.ai/core/go/pkg/core"
coreerr "forge.lthn.ai/core/go-log"
)

// Queries for git service
Expand Down Expand Up @@ -51,6 +51,9 @@ type ServiceOptions struct {
WorkDir string
}

// Compile-time interface checks.
var _ core.Startable = (*Service)(nil)

// Service provides git operations as a Core service.
type Service struct {
*core.ServiceRuntime[ServiceOptions]
Expand Down Expand Up @@ -137,14 +140,14 @@ func (s *Service) handleTask(c *core.Core, t core.Task) (any, bool, error) {

func (s *Service) validatePath(path string) error {
if !filepath.IsAbs(path) {
return fmt.Errorf("path must be absolute: %s", path)
return coreerr.E("git.validatePath", "path must be absolute: "+path, nil)
}

workDir := s.opts.WorkDir
if workDir != "" {
rel, err := filepath.Rel(workDir, path)
if err != nil || strings.HasPrefix(rel, "..") {
return fmt.Errorf("path %s is outside of allowed WorkDir %s", path, workDir)
return coreerr.E("git.validatePath", "path "+path+" is outside of allowed WorkDir "+workDir, nil)
}
}
return nil
Expand Down
Loading