refactor: remove unused workspace field from GitPushTool and CreatePRTool

Both tools get their working directory from worktree context, making the
workspace field redundant. Also replace custom contains helper with
strings.Contains in tests.

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

pkg/agent/instance.go

@@ -80,8 +80,8 @@ func NewAgentInstance(
 	toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
 	toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
 	toolsRegistry.Register(tools.NewLogsTool())
-	toolsRegistry.Register(tools.NewGitPushTool(workspace))
-	toolsRegistry.Register(tools.NewCreatePRTool(workspace))
+	toolsRegistry.Register(tools.NewGitPushTool())
+	toolsRegistry.Register(tools.NewCreatePRTool())
 
 	sessionsDir := filepath.Join(workspace, "sessions")
 	sessionsManager := session.NewSessionManager(sessionsDir)

pkg/tools/createpr.go

@@ -27,13 +27,12 @@ const (
 //   - If CI runs are triggered, a background goroutine polls `gh pr checks`
 //     and calls the AsyncCallback when CI completes (pass or fail)
 type CreatePRTool struct {
-	workspace string
 	callback AsyncCallback
 }
 
 // NewCreatePRTool creates a CreatePRTool.
-func NewCreatePRTool(workspace string) *CreatePRTool {
-	return &CreatePRTool{workspace: workspace}
+func NewCreatePRTool() *CreatePRTool {
+	return &CreatePRTool{}
 }
 
 func (t *CreatePRTool) Name() string { return "create_pr" }
@@ -151,7 +150,8 @@ func (t *CreatePRTool) Execute(ctx context.Context, args map[string]any) *ToolRe
 	}
 
 	// Build gh pr create command
-	ghArgs := []string{"pr", "create",
+	ghArgs := []string{
+		"pr", "create",
 		"--base", baseBranch,
 		"--head", branch,
 		"--title", title,

pkg/tools/createpr_test.go

@@ -2,6 +2,7 @@ package tools
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/sipeed/picoclaw/pkg/git"
@@ -9,7 +10,7 @@ import (
 
 // TestCreatePRTool_NoWorktree verifies that create_pr fails without worktree context.
 func TestCreatePRTool_NoWorktree(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 
 	result := tool.Execute(context.Background(), map[string]any{
 		"title": "Test PR",
@@ -23,7 +24,7 @@ func TestCreatePRTool_NoWorktree(t *testing.T) {
 
 // TestCreatePRTool_EmptyBranch verifies that empty branch name is rejected.
 func TestCreatePRTool_EmptyBranch(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 
 	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
 		Branch:     "",
@@ -42,7 +43,7 @@ func TestCreatePRTool_EmptyBranch(t *testing.T) {
 
 // TestCreatePRTool_MissingTitle verifies that missing title is rejected.
 func TestCreatePRTool_MissingTitle(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 
 	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
 		Branch:     "plan/test",
@@ -73,7 +74,7 @@ func TestCreatePRTool_MissingTitle(t *testing.T) {
 
 // TestCreatePRTool_BranchNotPushed verifies the tool checks for remote branch existence.
 func TestCreatePRTool_BranchNotPushed(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 
 	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
 		Branch:     "plan/not-pushed",
@@ -93,7 +94,7 @@ func TestCreatePRTool_BranchNotPushed(t *testing.T) {
 
 // TestCreatePRTool_DefaultBaseBranch verifies fallback to "main" when BaseBranch is empty.
 func TestCreatePRTool_DefaultBaseBranch(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 
 	// With empty BaseBranch, tool should default to "main"
 	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
@@ -106,7 +107,7 @@ func TestCreatePRTool_DefaultBaseBranch(t *testing.T) {
 		"title": "Test PR",
 	})
 	// Will fail at ls-remote (no real repo), but should not fail at baseBranch validation
-	if result.IsError && contains(result.ForLLM, "base branch") {
+	if result.IsError && strings.Contains(result.ForLLM, "base branch") {
 		t.Fatal("should not fail on base branch when defaulting to main")
 	}
 }
@@ -116,7 +117,7 @@ func TestCreatePRTool_Interface(t *testing.T) {
 	var _ Tool = (*CreatePRTool)(nil)
 	var _ AsyncTool = (*CreatePRTool)(nil)
 
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 	if tool.Name() != "create_pr" {
 		t.Errorf("Name: got %q, want %q", tool.Name(), "create_pr")
 	}
@@ -146,7 +147,7 @@ func TestCreatePRTool_Interface(t *testing.T) {
 
 // TestCreatePRTool_SetCallback verifies callback is stored.
 func TestCreatePRTool_SetCallback(t *testing.T) {
-	tool := NewCreatePRTool(t.TempDir())
+	tool := NewCreatePRTool()
 	if tool.callback != nil {
 		t.Fatal("callback should be nil initially")
 	}

pkg/tools/gitpush.go

@@ -38,13 +38,11 @@ var protectedBranches = regexp.MustCompile(`^(main|master|develop|release/.*)$`)
 //   - Protected branches (main, master, develop, release/*) are blocked
 //   - Force push is never allowed
 //   - Auto-commits uncommitted changes before pushing
-type GitPushTool struct {
-	workspace string
-}
+type GitPushTool struct{}
 
 // NewGitPushTool creates a GitPushTool.
-func NewGitPushTool(workspace string) *GitPushTool {
-	return &GitPushTool{workspace: workspace}
+func NewGitPushTool() *GitPushTool {
+	return &GitPushTool{}
 }
 
 func (t *GitPushTool) Name() string { return "git_push" }

pkg/tools/gitpush_test.go

@@ -2,6 +2,7 @@ package tools
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/sipeed/picoclaw/pkg/git"
@@ -9,7 +10,7 @@ import (
 
 // TestGitPushTool_NoWorktree verifies that git_push fails without worktree context.
 func TestGitPushTool_NoWorktree(t *testing.T) {
-	tool := NewGitPushTool(t.TempDir())
+	tool := NewGitPushTool()
 
 	result := tool.Execute(context.Background(), map[string]any{})
 	if !result.IsError {
@@ -25,7 +26,7 @@ func TestGitPushTool_NoWorktree(t *testing.T) {
 
 // TestGitPushTool_ProtectedBranch verifies that protected branches are blocked.
 func TestGitPushTool_ProtectedBranch(t *testing.T) {
-	tool := NewGitPushTool(t.TempDir())
+	tool := NewGitPushTool()
 
 	protectedNames := []string{"main", "master", "develop", "release/v1.0"}
 
@@ -49,7 +50,7 @@ func TestGitPushTool_ProtectedBranch(t *testing.T) {
 
 // TestGitPushTool_EmptyBranch verifies that empty branch name is rejected.
 func TestGitPushTool_EmptyBranch(t *testing.T) {
-	tool := NewGitPushTool(t.TempDir())
+	tool := NewGitPushTool()
 
 	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
 		Branch:     "",
@@ -67,7 +68,7 @@ func TestGitPushTool_EmptyBranch(t *testing.T) {
 // TestGitPushTool_AllowedBranch verifies that non-protected branches pass the branch check.
 // (Push itself will fail because there's no real git repo, but it should get past validation.)
 func TestGitPushTool_AllowedBranch(t *testing.T) {
-	tool := NewGitPushTool(t.TempDir())
+	tool := NewGitPushTool()
 
 	allowedNames := []string{"plan/add-auth", "feature/foo", "worktree/test"}
 
@@ -81,7 +82,7 @@ func TestGitPushTool_AllowedBranch(t *testing.T) {
 			})
 			result := tool.Execute(ctx, map[string]any{})
 			// Should NOT fail with "protected branch" error
-			if result.IsError && contains(result.ForLLM, "protected") {
+			if result.IsError && strings.Contains(result.ForLLM, "protected") {
 				t.Fatalf("branch %q should not be blocked as protected", branch)
 			}
 		})
@@ -147,7 +148,7 @@ func TestWorktreeInfoContext(t *testing.T) {
 func TestGitPushTool_Interface(t *testing.T) {
 	var _ Tool = (*GitPushTool)(nil)
 
-	tool := NewGitPushTool(t.TempDir())
+	tool := NewGitPushTool()
 	if tool.Name() != "git_push" {
 		t.Errorf("Name: got %q, want %q", tool.Name(), "git_push")
 	}
@@ -163,22 +164,9 @@ func TestGitPushTool_Interface(t *testing.T) {
 	}
 }
 
-func contains(s, substr string) bool {
-	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsStr(s, substr))
-}
-
-func containsStr(s, sub string) bool {
-	for i := 0; i+len(sub) <= len(s); i++ {
-		if s[i:i+len(sub)] == sub {
-			return true
-		}
-	}
-	return false
-}
-
 func assertContains(t *testing.T, s, substr string) {
 	t.Helper()
-	if !contains(s, substr) {
+	if !strings.Contains(s, substr) {
 		t.Errorf("expected %q to contain %q", s, substr)
 	}
 }

pkg/tools/subagent.go

@@ -360,10 +360,10 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string)
 
 	// Register git tools (worktree-safe push and PR creation)
 	if config.AllowedTools["git_push"] {
-		registry.Register(NewGitPushTool(sm.workspace))
+		registry.Register(NewGitPushTool())
 	}
 	if config.AllowedTools["create_pr"] {
-		registry.Register(NewCreatePRTool(sm.workspace))
+		registry.Register(NewCreatePRTool())
 	}
 
 	// Register web tools