fix: improve error handling and modernise with Go 1.26 features

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

.core/build.yaml

@@ -0,0 +1,24 @@
+version: 1
+
+project:
+  name: go-git
+  description: Git operations
+  binary: ""
+
+build:
+  cgo: false
+  flags:
+    - -trimpath
+  ldflags:
+    - -s
+    - -w
+
+targets:
+  - os: linux
+    arch: amd64
+  - os: linux
+    arch: arm64
+  - os: darwin
+    arch: arm64
+  - os: windows
+    arch: amd64

.core/release.yaml

@@ -0,0 +1,20 @@
+version: 1
+
+project:
+  name: go-git
+  repository: core/go-git
+
+publishers: []
+
+changelog:
+  include:
+    - feat
+    - fix
+    - perf
+    - refactor
+  exclude:
+    - chore
+    - docs
+    - style
+    - test
+    - ci

Taskfile.yml

@@ -1,46 +0,0 @@
-version: '3'
-
-tasks:
-  test:
-    desc: Run all tests
-    cmds:
-      - go test ./...
-
-  lint:
-    desc: Run golangci-lint
-    cmds:
-      - golangci-lint run ./...
-
-  fmt:
-    desc: Format all Go files
-    cmds:
-      - gofmt -w .
-
-  vet:
-    desc: Run go vet
-    cmds:
-      - go vet ./...
-
-  build:
-    desc: Build all Go packages
-    cmds:
-      - go build ./...
-
-  cov:
-    desc: Run tests with coverage and open HTML report
-    cmds:
-      - go test -coverprofile=coverage.out ./...
-      - go tool cover -html=coverage.out
-
-  tidy:
-    desc: Tidy go.mod
-    cmds:
-      - go mod tidy
-
-  check:
-    desc: Run fmt, vet, lint, and test in sequence
-    cmds:
-      - task: fmt
-      - task: vet
-      - task: lint
-      - task: test

git.go

@@ -4,9 +4,11 @@ package git
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
@@ -77,6 +79,12 @@ func getStatus(ctx context.Context, path, name string) RepoStatus {
 		Path: path,
 	}
 
+	// Validate path to prevent directory traversal
+	if !filepath.IsAbs(path) {
+		status.Error = fmt.Errorf("path must be absolute: %s", path)
+		return status
+	}
+
 	// Get current branch
 	branch, err := gitCommand(ctx, path, "rev-parse", "--abbrev-ref", "HEAD")
 	if err != nil {
@@ -117,7 +125,11 @@ func getStatus(ctx context.Context, path, name string) RepoStatus {
 	}
 
 	// Get ahead/behind counts
-	ahead, behind := getAheadBehind(ctx, path)
+	ahead, behind, err := getAheadBehind(ctx, path)
+	if err != nil {
+		// We don't fail the whole status if ahead/behind fails (might be no upstream)
+		// but we could log it or store it if needed. For now, we just keep 0.
+	}
 	status.Ahead = ahead
 	status.Behind = behind
 
@@ -125,20 +137,33 @@ func getStatus(ctx context.Context, path, name string) RepoStatus {
 }
 
 // getAheadBehind returns the number of commits ahead and behind upstream.
-func getAheadBehind(ctx context.Context, path string) (ahead, behind int) {
+func getAheadBehind(ctx context.Context, path string) (ahead, behind int, err error) {
 	// Try to get ahead count
 	aheadStr, err := gitCommand(ctx, path, "rev-list", "--count", "@{u}..HEAD")
 	if err == nil {
 		ahead, _ = strconv.Atoi(strings.TrimSpace(aheadStr))
+	} else {
+		// If it failed because of no upstream, don't return error
+		if strings.Contains(err.Error(), "no upstream") || strings.Contains(err.Error(), "No upstream") {
+			err = nil
+		}
+	}
+
+	if err != nil {
+		return 0, 0, err
 	}
 
 	// Try to get behind count
 	behindStr, err := gitCommand(ctx, path, "rev-list", "--count", "HEAD..@{u}")
 	if err == nil {
 		behind, _ = strconv.Atoi(strings.TrimSpace(behindStr))
+	} else {
+		if strings.Contains(err.Error(), "no upstream") || strings.Contains(err.Error(), "No upstream") {
+			err = nil
+		}
 	}
 
-	return ahead, behind
+	return ahead, behind, err
 }
 
 // Push pushes commits for a single repository.
@@ -178,10 +203,11 @@ func gitInteractive(ctx context.Context, dir string, args ...string) error {
 	cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
 
 	if err := cmd.Run(); err != nil {
-		if stderr.Len() > 0 {
-			return &GitError{Err: err, Stderr: stderr.String()}
+		return &GitError{
+			Args:   args,
+			Err:    err,
+			Stderr: stderr.String(),
 		}
-		return err
 	}
 
 	return nil
@@ -197,8 +223,9 @@ type PushResult struct {
 
 // PushMultiple pushes multiple repositories sequentially.
 // Sequential because SSH passphrase prompts need user interaction.
-func PushMultiple(ctx context.Context, paths []string, names map[string]string) []PushResult {
+func PushMultiple(ctx context.Context, paths []string, names map[string]string) ([]PushResult, error) {
 	results := make([]PushResult, len(paths))
+	var lastErr error
 
 	for i, path := range paths {
 		name := names[path]
@@ -214,6 +241,7 @@ func PushMultiple(ctx context.Context, paths []string, names map[string]string)
 		err := Push(ctx, path)
 		if err != nil {
 			result.Error = err
+			lastErr = err
 		} else {
 			result.Success = true
 		}
@@ -221,7 +249,7 @@ func PushMultiple(ctx context.Context, paths []string, names map[string]string)
 		results[i] = result
 	}
 
-	return results
+	return results, lastErr
 }
 
 // gitCommand runs a git command and returns stdout.
@@ -234,30 +262,32 @@ func gitCommand(ctx context.Context, dir string, args ...string) (string, error)
 	cmd.Stderr = &stderr
 
 	if err := cmd.Run(); err != nil {
-		// Include stderr in error message for better diagnostics
-		if stderr.Len() > 0 {
-			return "", &GitError{Err: err, Stderr: stderr.String()}
+		return "", &GitError{
+			Args:   args,
+			Err:    err,
+			Stderr: stderr.String(),
 		}
-		return "", err
 	}
 
 	return stdout.String(), nil
 }
 
-// GitError wraps a git command error with stderr output.
+// GitError wraps a git command error with stderr output and command context.
 type GitError struct {
+	Args   []string
 	Err    error
 	Stderr string
 }
 
-// Error returns the git error message, preferring stderr output.
+// Error returns a descriptive error message.
 func (e *GitError) Error() string {
-	// Return just the stderr message, trimmed
-	msg := strings.TrimSpace(e.Stderr)
-	if msg != "" {
-		return msg
+	cmd := "git " + strings.Join(e.Args, " ")
+	stderr := strings.TrimSpace(e.Stderr)
+
+	if stderr != "" {
+		return fmt.Errorf("git command %q failed: %s", cmd, stderr).Error()
 	}
-	return e.Err.Error()
+	return fmt.Errorf("git command %q failed: %w", cmd, e.Err).Error()
 }
 
 // Unwrap returns the underlying error for error chain inspection.

git_test.go

@@ -169,18 +169,13 @@ func TestGitError_Error(t *testing.T) {
 	}{
 		{
 			name:     "stderr takes precedence",
-			err:      &GitError{Err: errors.New("exit 1"), Stderr: "fatal: not a git repository"},
-			expected: "fatal: not a git repository",
+			err:      &GitError{Args: []string{"status"}, Err: errors.New("exit 1"), Stderr: "fatal: not a git repository"},
+			expected: "git command \"git status\" failed: fatal: not a git repository",
 		},
 		{
 			name:     "falls back to underlying error",
-			err:      &GitError{Err: errors.New("exit status 128"), Stderr: ""},
-			expected: "exit status 128",
-		},
-		{
-			name:     "trims whitespace from stderr",
-			err:      &GitError{Err: errors.New("exit 1"), Stderr: "  error message  \n"},
-			expected: "error message",
+			err:      &GitError{Args: []string{"status"}, Err: errors.New("exit status 128"), Stderr: ""},
+			expected: "git command \"git status\" failed: exit status 128",
 		},
 	}
 
@@ -243,7 +238,7 @@ func TestIsNonFastForward(t *testing.T) {
 // --- gitCommand tests with real git repos ---
 
 func TestGitCommand_Good(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	out, err := gitCommand(context.Background(), dir, "rev-parse", "--abbrev-ref", "HEAD")
 	require.NoError(t, err)
@@ -258,7 +253,7 @@ func TestGitCommand_Bad_InvalidDir(t *testing.T) {
 }
 
 func TestGitCommand_Bad_NotARepo(t *testing.T) {
-	dir := t.TempDir()
+	dir, _ := filepath.Abs(t.TempDir())
 	_, err := gitCommand(context.Background(), dir, "status")
 	require.Error(t, err)
 
@@ -266,13 +261,14 @@ func TestGitCommand_Bad_NotARepo(t *testing.T) {
 	var gitErr *GitError
 	if errors.As(err, &gitErr) {
 		assert.Contains(t, gitErr.Stderr, "not a git repository")
+		assert.Equal(t, []string{"status"}, gitErr.Args)
 	}
 }
 
 // --- getStatus integration tests ---
 
 func TestGetStatus_Good_CleanRepo(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	status := getStatus(context.Background(), dir, "test-repo")
 	require.NoError(t, status.Error)
@@ -283,7 +279,7 @@ func TestGetStatus_Good_CleanRepo(t *testing.T) {
 }
 
 func TestGetStatus_Good_ModifiedFile(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Modify the existing tracked file.
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("# Modified\n"), 0644))
@@ -295,7 +291,7 @@ func TestGetStatus_Good_ModifiedFile(t *testing.T) {
 }
 
 func TestGetStatus_Good_UntrackedFile(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Create a new untracked file.
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "newfile.txt"), []byte("hello"), 0644))
@@ -307,7 +303,7 @@ func TestGetStatus_Good_UntrackedFile(t *testing.T) {
 }
 
 func TestGetStatus_Good_StagedFile(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Create and stage a new file.
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "staged.txt"), []byte("staged"), 0644))
@@ -322,7 +318,7 @@ func TestGetStatus_Good_StagedFile(t *testing.T) {
 }
 
 func TestGetStatus_Good_MixedChanges(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Create untracked file.
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "untracked.txt"), []byte("new"), 0644))
@@ -345,7 +341,7 @@ func TestGetStatus_Good_MixedChanges(t *testing.T) {
 }
 
 func TestGetStatus_Good_DeletedTrackedFile(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Delete the tracked file (unstaged deletion).
 	require.NoError(t, os.Remove(filepath.Join(dir, "README.md")))
@@ -357,7 +353,7 @@ func TestGetStatus_Good_DeletedTrackedFile(t *testing.T) {
 }
 
 func TestGetStatus_Good_StagedDeletion(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Stage a deletion.
 	cmd := exec.Command("git", "rm", "README.md")
@@ -379,8 +375,8 @@ func TestGetStatus_Bad_InvalidPath(t *testing.T) {
 // --- Status (parallel multi-repo) tests ---
 
 func TestStatus_Good_MultipleRepos(t *testing.T) {
-	dir1 := initTestRepo(t)
-	dir2 := initTestRepo(t)
+	dir1, _ := filepath.Abs(initTestRepo(t))
+	dir2, _ := filepath.Abs(initTestRepo(t))
 
 	// Make dir2 dirty.
 	require.NoError(t, os.WriteFile(filepath.Join(dir2, "extra.txt"), []byte("extra"), 0644))
@@ -412,7 +408,7 @@ func TestStatus_Good_EmptyPaths(t *testing.T) {
 }
 
 func TestStatus_Good_NameFallback(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// No name mapping — path should be used as name.
 	results := Status(context.Background(), StatusOptions{
@@ -425,8 +421,8 @@ func TestStatus_Good_NameFallback(t *testing.T) {
 }
 
 func TestStatus_Good_WithErrors(t *testing.T) {
-	validDir := initTestRepo(t)
-	invalidDir := "/nonexistent/path"
+	validDir, _ := filepath.Abs(initTestRepo(t))
+	invalidDir, _ := filepath.Abs("/nonexistent/path")
 
 	results := Status(context.Background(), StatusOptions{
 		Paths: []string{validDir, invalidDir},
@@ -445,11 +441,12 @@ func TestStatus_Good_WithErrors(t *testing.T) {
 
 func TestPushMultiple_Good_NoRemote(t *testing.T) {
 	// Push without a remote will fail but we can test the result structure.
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	results := PushMultiple(context.Background(), []string{dir}, map[string]string{
+	results, err := PushMultiple(context.Background(), []string{dir}, map[string]string{
 		dir: "test-repo",
 	})
+	assert.Error(t, err)
 
 	require.Len(t, results, 1)
 	assert.Equal(t, "test-repo", results[0].Name)
@@ -460,9 +457,10 @@ func TestPushMultiple_Good_NoRemote(t *testing.T) {
 }
 
 func TestPushMultiple_Good_NameFallback(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	results := PushMultiple(context.Background(), []string{dir}, map[string]string{})
+	results, err := PushMultiple(context.Background(), []string{dir}, map[string]string{})
+	assert.Error(t, err)
 
 	require.Len(t, results, 1)
 	assert.Equal(t, dir, results[0].Name, "name should fall back to path")
@@ -471,7 +469,7 @@ func TestPushMultiple_Good_NameFallback(t *testing.T) {
 // --- Pull tests ---
 
 func TestPull_Bad_NoRemote(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 	err := Pull(context.Background(), dir)
 	assert.Error(t, err, "pull without remote should fail")
 }
@@ -479,7 +477,7 @@ func TestPull_Bad_NoRemote(t *testing.T) {
 // --- Push tests ---
 
 func TestPush_Bad_NoRemote(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 	err := Push(context.Background(), dir)
 	assert.Error(t, err, "push without remote should fail")
 }
@@ -487,7 +485,7 @@ func TestPush_Bad_NoRemote(t *testing.T) {
 // --- Context cancellation test ---
 
 func TestGetStatus_Good_ContextCancellation(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // Cancel immediately.
@@ -501,8 +499,8 @@ func TestGetStatus_Good_ContextCancellation(t *testing.T) {
 
 func TestGetAheadBehind_Good_WithUpstream(t *testing.T) {
 	// Create a bare remote and a clone to test ahead/behind counts.
-	bareDir := t.TempDir()
-	cloneDir := t.TempDir()
+	bareDir, _ := filepath.Abs(t.TempDir())
+	cloneDir, _ := filepath.Abs(t.TempDir())
 
 	// Initialise the bare repo.
 	cmd := exec.Command("git", "init", "--bare")
@@ -547,7 +545,8 @@ func TestGetAheadBehind_Good_WithUpstream(t *testing.T) {
 		require.NoError(t, cmd.Run())
 	}
 
-	ahead, behind := getAheadBehind(context.Background(), cloneDir)
+	ahead, behind, err := getAheadBehind(context.Background(), cloneDir)
+	assert.NoError(t, err)
 	assert.Equal(t, 1, ahead, "should be 1 commit ahead")
 	assert.Equal(t, 0, behind, "should not be behind")
 }
@@ -555,7 +554,7 @@ func TestGetAheadBehind_Good_WithUpstream(t *testing.T) {
 // --- Renamed file detection ---
 
 func TestGetStatus_Good_RenamedFile(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	// Rename via git mv (stages the rename).
 	cmd := exec.Command("git", "mv", "README.md", "GUIDE.md")

go.mod

@@ -2,14 +2,13 @@ module forge.lthn.ai/core/go-git
 
 go 1.26.0
 
-require (
-	forge.lthn.ai/core/go v0.0.9
-	github.com/stretchr/testify v1.11.1
-)
+require 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/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
 )

go.sum

@@ -1,5 +1,3 @@
-forge.lthn.ai/core/go v0.0.9 h1:f1FlnFGBvV280N+rI0MEejNT7yNt42PE3Nm9kHE73Rw=
-forge.lthn.ai/core/go v0.0.9/go.mod h1:k3dpMA1jzxIiuFrwmZUzK3cMZd5xQRmPiYI7DInFJug=
 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=

service.go

@@ -2,10 +2,14 @@ package git
 
 import (
 	"context"
+	"fmt"
 	"iter"
+	"path/filepath"
 	"slices"
+	"strings"
+	"sync"
 
-	"forge.lthn.ai/core/go/pkg/framework"
+	"forge.lthn.ai/core/go/pkg/core"
 )
 
 // Queries for git service
@@ -49,15 +53,18 @@ type ServiceOptions struct {
 
 // Service provides git operations as a Core service.
 type Service struct {
-	*framework.ServiceRuntime[ServiceOptions]
+	*core.ServiceRuntime[ServiceOptions]
+	opts       ServiceOptions
+	mu         sync.RWMutex
 	lastStatus []RepoStatus
 }
 
 // NewService creates a git service factory.
-func NewService(opts ServiceOptions) func(*framework.Core) (any, error) {
-	return func(c *framework.Core) (any, error) {
+func NewService(opts ServiceOptions) func(*core.Core) (any, error) {
+	return func(c *core.Core) (any, error) {
 		return &Service{
-			ServiceRuntime: framework.NewServiceRuntime(c, opts),
+			ServiceRuntime: core.NewServiceRuntime(c, opts),
+			opts:           opts,
 		}, nil
 	}
 }
@@ -69,11 +76,24 @@ func (s *Service) OnStartup(ctx context.Context) error {
 	return nil
 }
 
-func (s *Service) handleQuery(c *framework.Core, q framework.Query) (any, bool, error) {
+func (s *Service) handleQuery(c *core.Core, q core.Query) (any, bool, error) {
+	ctx := context.Background() // TODO: core should pass context to handlers
+
 	switch m := q.(type) {
 	case QueryStatus:
-		statuses := Status(context.Background(), StatusOptions(m))
+		// Validate all paths before execution
+		for _, path := range m.Paths {
+			if err := s.validatePath(path); err != nil {
+				return nil, true, err
+			}
+		}
+
+		statuses := Status(ctx, StatusOptions(m))
+
+		s.mu.Lock()
 		s.lastStatus = statuses
+		s.mu.Unlock()
+
 		return statuses, true, nil
 
 	case QueryDirtyRepos:
@@ -85,35 +105,73 @@ func (s *Service) handleQuery(c *framework.Core, q framework.Query) (any, bool,
 	return nil, false, nil
 }
 
-func (s *Service) handleTask(c *framework.Core, t framework.Task) (any, bool, error) {
+func (s *Service) handleTask(c *core.Core, t core.Task) (any, bool, error) {
+	ctx := context.Background() // TODO: core should pass context to handlers
+
 	switch m := t.(type) {
 	case TaskPush:
-		err := Push(context.Background(), m.Path)
+		if err := s.validatePath(m.Path); err != nil {
+			return nil, true, err
+		}
+		err := Push(ctx, m.Path)
 		return nil, true, err
 
 	case TaskPull:
-		err := Pull(context.Background(), m.Path)
+		if err := s.validatePath(m.Path); err != nil {
+			return nil, true, err
+		}
+		err := Pull(ctx, m.Path)
 		return nil, true, err
 
 	case TaskPushMultiple:
-		results := PushMultiple(context.Background(), m.Paths, m.Names)
-		return results, true, nil
+		for _, path := range m.Paths {
+			if err := s.validatePath(path); err != nil {
+				return nil, true, err
+			}
+		}
+		results, err := PushMultiple(ctx, m.Paths, m.Names)
+		return results, true, err
 	}
 	return nil, false, nil
 }
 
+func (s *Service) validatePath(path string) error {
+	if !filepath.IsAbs(path) {
+		return fmt.Errorf("path must be absolute: %s", path)
+	}
+
+	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 nil
+}
+
 // Status returns last status result.
-func (s *Service) Status() []RepoStatus { return s.lastStatus }
+func (s *Service) Status() []RepoStatus {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return slices.Clone(s.lastStatus)
+}
 
 // All returns an iterator over all last known statuses.
 func (s *Service) All() iter.Seq[RepoStatus] {
-	return slices.Values(s.lastStatus)
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return slices.Values(slices.Clone(s.lastStatus))
 }
 
 // Dirty returns an iterator over repos with uncommitted changes.
 func (s *Service) Dirty() iter.Seq[RepoStatus] {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	lastStatus := slices.Clone(s.lastStatus)
+
 	return func(yield func(RepoStatus) bool) {
-		for _, st := range s.lastStatus {
+		for _, st := range lastStatus {
 			if st.Error == nil && st.IsDirty() {
 				if !yield(st) {
 					return
@@ -125,8 +183,12 @@ func (s *Service) Dirty() iter.Seq[RepoStatus] {
 
 // Ahead returns an iterator over repos with unpushed commits.
 func (s *Service) Ahead() iter.Seq[RepoStatus] {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	lastStatus := slices.Clone(s.lastStatus)
+
 	return func(yield func(RepoStatus) bool) {
-		for _, st := range s.lastStatus {
+		for _, st := range lastStatus {
 			if st.Error == nil && st.HasUnpushed() {
 				if !yield(st) {
 					return

service_extra_test.go

@@ -10,16 +10,16 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"forge.lthn.ai/core/go/pkg/framework"
+	"forge.lthn.ai/core/go/pkg/core"
 )
 
 func TestNewService_Good(t *testing.T) {
-	opts := ServiceOptions{WorkDir: "/tmp/test"}
+	opts := ServiceOptions{WorkDir: t.TempDir()}
 	factory := NewService(opts)
 	assert.NotNil(t, factory)
 
 	// Create a minimal Core to test the factory.
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc, err := factory(c)
@@ -32,12 +32,13 @@ func TestNewService_Good(t *testing.T) {
 }
 
 func TestService_OnStartup_Good(t *testing.T) {
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
-	opts := ServiceOptions{WorkDir: "/tmp"}
+	opts := ServiceOptions{WorkDir: t.TempDir()}
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, opts),
+		ServiceRuntime: core.NewServiceRuntime(c, opts),
+		opts:           opts,
 	}
 
 	err = svc.OnStartup(context.Background())
@@ -45,13 +46,13 @@ func TestService_OnStartup_Good(t *testing.T) {
 }
 
 func TestService_HandleQuery_Good_Status(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	// Call handleQuery directly.
@@ -73,11 +74,11 @@ func TestService_HandleQuery_Good_Status(t *testing.T) {
 }
 
 func TestService_HandleQuery_Good_DirtyRepos(t *testing.T) {
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 		lastStatus: []RepoStatus{
 			{Name: "clean"},
 			{Name: "dirty", Modified: 1},
@@ -95,11 +96,11 @@ func TestService_HandleQuery_Good_DirtyRepos(t *testing.T) {
 }
 
 func TestService_HandleQuery_Good_AheadRepos(t *testing.T) {
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 		lastStatus: []RepoStatus{
 			{Name: "synced"},
 			{Name: "ahead", Ahead: 3},
@@ -117,11 +118,11 @@ func TestService_HandleQuery_Good_AheadRepos(t *testing.T) {
 }
 
 func TestService_HandleQuery_Good_UnknownQuery(t *testing.T) {
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	result, handled, err := svc.handleQuery(c, "unknown query type")
@@ -131,13 +132,13 @@ func TestService_HandleQuery_Good_UnknownQuery(t *testing.T) {
 }
 
 func TestService_HandleTask_Good_Push(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	// Push without a remote will fail, but handleTask should still handle it.
@@ -147,13 +148,13 @@ func TestService_HandleTask_Good_Push(t *testing.T) {
 }
 
 func TestService_HandleTask_Good_Pull(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	_, handled, err := svc.handleTask(c, TaskPull{Path: dir, Name: "test"})
@@ -162,13 +163,13 @@ func TestService_HandleTask_Good_Pull(t *testing.T) {
 }
 
 func TestService_HandleTask_Good_PushMultiple(t *testing.T) {
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	result, handled, err := svc.handleTask(c, TaskPushMultiple{
@@ -177,7 +178,7 @@ func TestService_HandleTask_Good_PushMultiple(t *testing.T) {
 	})
 
 	assert.True(t, handled)
-	assert.NoError(t, err) // PushMultiple does not return errors directly
+	assert.Error(t, err) // PushMultiple returns errors directly
 
 	results, ok := result.([]PushResult)
 	require.True(t, ok)
@@ -186,11 +187,11 @@ func TestService_HandleTask_Good_PushMultiple(t *testing.T) {
 }
 
 func TestService_HandleTask_Good_UnknownTask(t *testing.T) {
-	c, err := framework.New()
+	c, err := core.New()
 	require.NoError(t, err)
 
 	svc := &Service{
-		ServiceRuntime: framework.NewServiceRuntime(c, ServiceOptions{}),
+		ServiceRuntime: core.NewServiceRuntime(c, ServiceOptions{}),
 	}
 
 	result, handled, err := svc.handleTask(c, "unknown task")
@@ -203,7 +204,7 @@ func TestService_HandleTask_Good_UnknownTask(t *testing.T) {
 
 func TestGetStatus_Good_AheadBehindNoUpstream(t *testing.T) {
 	// A repo without a tracking branch should return 0 ahead/behind.
-	dir := initTestRepo(t)
+	dir, _ := filepath.Abs(initTestRepo(t))
 
 	status := getStatus(context.Background(), dir, "no-upstream")
 	require.NoError(t, status.Error)
@@ -212,18 +213,20 @@ func TestGetStatus_Good_AheadBehindNoUpstream(t *testing.T) {
 }
 
 func TestPushMultiple_Good_Empty(t *testing.T) {
-	results := PushMultiple(context.Background(), []string{}, map[string]string{})
+	results, err := PushMultiple(context.Background(), []string{}, map[string]string{})
+	assert.NoError(t, err)
 	assert.Empty(t, results)
 }
 
 func TestPushMultiple_Good_MultiplePaths(t *testing.T) {
-	dir1 := initTestRepo(t)
-	dir2 := initTestRepo(t)
+	dir1, _ := filepath.Abs(initTestRepo(t))
+	dir2, _ := filepath.Abs(initTestRepo(t))
 
-	results := PushMultiple(context.Background(), []string{dir1, dir2}, map[string]string{
+	results, err := PushMultiple(context.Background(), []string{dir1, dir2}, map[string]string{
 		dir1: "repo-1",
 		dir2: "repo-2",
 	})
+	assert.Error(t, err)
 
 	require.Len(t, results, 2)
 	assert.Equal(t, "repo-1", results[0].Name)
@@ -235,8 +238,8 @@ func TestPushMultiple_Good_MultiplePaths(t *testing.T) {
 
 func TestPush_Good_WithRemote(t *testing.T) {
 	// Create a bare remote and a clone.
-	bareDir := t.TempDir()
-	cloneDir := t.TempDir()
+	bareDir, _ := filepath.Abs(t.TempDir())
+	cloneDir, _ := filepath.Abs(t.TempDir())
 
 	cmd := exec.Command("git", "init", "--bare")
 	cmd.Dir = bareDir
@@ -282,6 +285,8 @@ func TestPush_Good_WithRemote(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Verify ahead count is now 0.
-	ahead, _ := getAheadBehind(context.Background(), cloneDir)
+	ahead, behind, err := getAheadBehind(context.Background(), cloneDir)
+	assert.NoError(t, err)
 	assert.Equal(t, 0, ahead)
+	assert.Equal(t, 0, behind)
 }