From 2224a062fa71e43131c00ed5eb513b4061e6658e Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 1 Mar 2026 06:20:59 +0000
Subject: [PATCH 1/4] feat: add git_push and create_pr tools for worktree-safe
 git operations

Adds two dedicated tools that bypass the shell exec deny patterns while
maintaining strict safety invariants:

- git_push: pushes the current worktree branch to origin. Requires active
  worktree context, blocks protected branches (main/master/develop/release/*),
  forbids force push, auto-commits uncommitted changes before pushing.

- create_pr: creates a GitHub PR via `gh` CLI. Auto-detects base branch from
  worktree info, verifies branch is pushed, checks for merge conflicts with
  base via `git merge-tree --write-tree` before creating.

Sandbox integration:
- coder+ presets: git_push allowed
- worker+ presets: create_pr also allowed
- WorktreeInfo injected into tool context alongside workspace override

https://claude.ai/code/session_01WWttNE5xShanYD6PhMzgKz
---
 pkg/agent/instance.go      |   2 +
 pkg/agent/loop.go          |   1 +
 pkg/tools/createpr.go      | 175 +++++++++++++++++++++++++++++++++++
 pkg/tools/createpr_test.go | 173 ++++++++++++++++++++++++++++++++++
 pkg/tools/gitpush.go       | 144 +++++++++++++++++++++++++++++
 pkg/tools/gitpush_test.go  | 184 +++++++++++++++++++++++++++++++++++++
 pkg/tools/sandbox.go       |   8 +-
 pkg/tools/subagent.go      |   8 ++
 8 files changed, 694 insertions(+), 1 deletion(-)
 create mode 100644 pkg/tools/createpr.go
 create mode 100644 pkg/tools/createpr_test.go
 create mode 100644 pkg/tools/gitpush.go
 create mode 100644 pkg/tools/gitpush_test.go

diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go
index 1ba2faea1..a52024dd2 100644
--- a/pkg/agent/instance.go
+++ b/pkg/agent/instance.go
@@ -80,6 +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))
 
 	sessionsDir := filepath.Join(workspace, "sessions")
 	sessionsManager := session.NewSessionManager(sessionsDir)
diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go
index 0d8f71bb1..2417597a3 100644
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -2637,6 +2637,7 @@ func (al *AgentLoop) runLLMIteration(
 			toolCtx := ctx
 			if wt := agent.GetWorktree(opts.SessionKey); wt != nil {
 				toolCtx = tools.WithWorkspaceOverride(toolCtx, wt.Path)
+				toolCtx = tools.WithWorktreeInfo(toolCtx, wt)
 			}
 			toolResult := agent.Tools.ExecuteWithContext(
 				toolCtx,
diff --git a/pkg/tools/createpr.go b/pkg/tools/createpr.go
new file mode 100644
index 000000000..165d1f833
--- /dev/null
+++ b/pkg/tools/createpr.go
@@ -0,0 +1,175 @@
+package tools
+
+import (
+	"context"
+	"fmt"
+	"os/exec"
+	"strings"
+	"time"
+)
+
+// CreatePRTool creates a GitHub pull request from the current worktree branch.
+//
+// Safety invariants:
+//   - Only works inside a worktree (WorktreeInfo must be in context)
+//   - Base branch is auto-detected from WorktreeInfo.BaseBranch
+//   - Requires the branch to be already pushed (use git_push first)
+//   - Checks for merge conflicts with base before creating
+//   - Uses `gh pr create` under the hood
+type CreatePRTool struct {
+	workspace string
+}
+
+// NewCreatePRTool creates a CreatePRTool.
+func NewCreatePRTool(workspace string) *CreatePRTool {
+	return &CreatePRTool{workspace: workspace}
+}
+
+func (t *CreatePRTool) Name() string { return "create_pr" }
+
+func (t *CreatePRTool) Description() string {
+	return "Create a GitHub pull request from the current worktree branch. " +
+		"The base branch is auto-detected from the worktree's parent branch. " +
+		"The branch must be pushed to origin first (use git_push). " +
+		"Checks for merge conflicts with the base branch before creating. " +
+		"Requires the `gh` CLI to be installed and authenticated."
+}
+
+func (t *CreatePRTool) Parameters() map[string]any {
+	return map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"title": map[string]any{
+				"type":        "string",
+				"description": "Pull request title",
+			},
+			"body": map[string]any{
+				"type":        "string",
+				"description": "Pull request body/description (supports markdown)",
+			},
+			"draft": map[string]any{
+				"type":        "boolean",
+				"description": "Create as draft PR (default: false)",
+			},
+		},
+		"required": []string{"title"},
+	}
+}
+
+func (t *CreatePRTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
+	wt := WorktreeInfoFromCtx(ctx)
+	if wt == nil {
+		return ErrorResult(
+			"create_pr requires an active worktree.\n" +
+				"This tool can only be used during worktree-based sessions " +
+				"(e.g., heartbeat tasks or plan executing phase).\n" +
+				"The worktree provides the branch name and base branch for the PR.")
+	}
+
+	branch := wt.Branch
+	if branch == "" {
+		return ErrorResult(
+			"worktree has no branch name.\n" +
+				"The WorktreeInfo was set but Branch is empty. " +
+				"This is an internal error — the worktree may not have been created correctly.")
+	}
+
+	baseBranch := wt.BaseBranch
+	if baseBranch == "" {
+		baseBranch = "main"
+	}
+
+	title, ok := args["title"].(string)
+	if !ok || strings.TrimSpace(title) == "" {
+		return ErrorResult(
+			"title is required.\n" +
+				"Provide a concise PR title describing the change (e.g., \"Add rate limiter to API endpoints\").")
+	}
+
+	// Verify the branch has been pushed by checking if the remote ref exists
+	checkCtx, checkCancel := context.WithTimeout(ctx, 15*time.Second)
+	defer checkCancel()
+	checkCmd := exec.CommandContext(checkCtx, "git", "ls-remote", "--exit-code", "origin", branch)
+	checkCmd.Dir = wt.Path
+	if err := checkCmd.Run(); err != nil {
+		return ErrorResult(fmt.Sprintf(
+			"branch %q not found on origin.\n"+
+				"The branch must be pushed before creating a PR. Use the git_push tool first.\n"+
+				"git_push will auto-commit uncommitted changes and push the worktree branch to origin.",
+			branch))
+	}
+
+	// Fetch latest base branch and check for merge conflicts
+	fetchCtx, fetchCancel := context.WithTimeout(ctx, 30*time.Second)
+	defer fetchCancel()
+	fetchCmd := exec.CommandContext(fetchCtx, "git", "fetch", "origin", baseBranch)
+	fetchCmd.Dir = wt.Path
+	if out, err := fetchCmd.CombinedOutput(); err != nil {
+		return ErrorResult(fmt.Sprintf(
+			"failed to fetch origin/%s: %s\n%s\n"+
+				"Cannot verify merge compatibility without the latest base branch. "+
+				"Check network connectivity and that the base branch %q exists on origin.",
+			baseBranch, err, strings.TrimSpace(string(out)), baseBranch))
+	}
+
+	// Try a merge dry-run to detect conflicts.
+	// merge-tree --write-tree is a plumbing command (Git 2.38+) that performs a
+	// three-way merge entirely in-memory without touching the working tree.
+	// Exit code 0 = clean merge, non-zero = conflicts detected.
+	mergeCtx, mergeCancel := context.WithTimeout(ctx, 30*time.Second)
+	defer mergeCancel()
+	mergeCmd := exec.CommandContext(mergeCtx, "git", "merge-tree",
+		"--write-tree", "--no-messages",
+		branch, "origin/"+baseBranch)
+	mergeCmd.Dir = wt.RepoRoot
+	mergeOut, mergeErr := mergeCmd.CombinedOutput()
+	if mergeErr != nil {
+		conflictInfo := strings.TrimSpace(string(mergeOut))
+		return ErrorResult(fmt.Sprintf(
+			"merge conflict detected between %q and %s.\n"+
+				"The PR cannot be created cleanly. Resolve the conflicts in the worktree first, "+
+				"then use git_push to push the resolution before retrying create_pr.\n"+
+				"Conflict details:\n%s",
+			branch, baseBranch, conflictInfo))
+	}
+
+	// Build gh pr create command
+	ghArgs := []string{"pr", "create",
+		"--base", baseBranch,
+		"--head", branch,
+		"--title", title,
+	}
+
+	if body, ok := args["body"].(string); ok && body != "" {
+		ghArgs = append(ghArgs, "--body", body)
+	} else {
+		ghArgs = append(ghArgs, "--body", "")
+	}
+
+	if draft, ok := args["draft"].(bool); ok && draft {
+		ghArgs = append(ghArgs, "--draft")
+	}
+
+	prCtx, prCancel := context.WithTimeout(ctx, 30*time.Second)
+	defer prCancel()
+
+	cmd := exec.CommandContext(prCtx, "gh", ghArgs...)
+	cmd.Dir = wt.RepoRoot
+	out, err := cmd.CombinedOutput()
+	output := strings.TrimSpace(string(out))
+
+	if err != nil {
+		return ErrorResult(fmt.Sprintf(
+			"gh pr create failed: %s\n%s\n"+
+				"Possible causes:\n"+
+				"- gh CLI not installed or not authenticated (run `gh auth login`)\n"+
+				"- A PR already exists for branch %q (check with `gh pr list`)\n"+
+				"- Repository not configured as a GitHub remote",
+			err, output, branch))
+	}
+
+	return NewToolResult(fmt.Sprintf(
+		"Pull request created: %s\n"+
+			"Branch: %s -> %s",
+		output, branch, baseBranch))
+}
diff --git a/pkg/tools/createpr_test.go b/pkg/tools/createpr_test.go
new file mode 100644
index 000000000..e9392ef88
--- /dev/null
+++ b/pkg/tools/createpr_test.go
@@ -0,0 +1,173 @@
+package tools
+
+import (
+	"context"
+	"testing"
+
+	"github.com/sipeed/picoclaw/pkg/git"
+)
+
+// TestCreatePRTool_NoWorktree verifies that create_pr fails without worktree context.
+func TestCreatePRTool_NoWorktree(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+
+	result := tool.Execute(context.Background(), map[string]any{
+		"title": "Test PR",
+	})
+	if !result.IsError {
+		t.Fatal("expected error when no worktree in context")
+	}
+	assertContains(t, result.ForLLM, "worktree")
+	assertContains(t, result.ForLLM, "heartbeat")
+}
+
+// TestCreatePRTool_EmptyBranch verifies that empty branch name is rejected.
+func TestCreatePRTool_EmptyBranch(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+
+	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+		Branch:     "",
+		BaseBranch: "main",
+		Path:       t.TempDir(),
+		RepoRoot:   t.TempDir(),
+	})
+	result := tool.Execute(ctx, map[string]any{
+		"title": "Test PR",
+	})
+	if !result.IsError {
+		t.Fatal("expected error for empty branch")
+	}
+	assertContains(t, result.ForLLM, "no branch name")
+}
+
+// TestCreatePRTool_MissingTitle verifies that missing title is rejected.
+func TestCreatePRTool_MissingTitle(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+
+	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+		Branch:     "plan/test",
+		BaseBranch: "main",
+		Path:       t.TempDir(),
+		RepoRoot:   t.TempDir(),
+	})
+
+	tests := []struct {
+		name string
+		args map[string]any
+	}{
+		{"no title key", map[string]any{}},
+		{"empty title", map[string]any{"title": ""}},
+		{"whitespace title", map[string]any{"title": "   "}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tool.Execute(ctx, tt.args)
+			if !result.IsError {
+				t.Fatal("expected error for missing/empty title")
+			}
+			assertContains(t, result.ForLLM, "title is required")
+		})
+	}
+}
+
+// TestCreatePRTool_BranchNotPushed verifies the tool checks for remote branch existence.
+func TestCreatePRTool_BranchNotPushed(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+
+	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+		Branch:     "plan/not-pushed",
+		BaseBranch: "main",
+		Path:       t.TempDir(),
+		RepoRoot:   t.TempDir(),
+	})
+	result := tool.Execute(ctx, map[string]any{
+		"title": "Test PR",
+	})
+	if !result.IsError {
+		t.Fatal("expected error for unpushed branch")
+	}
+	// Should mention git_push as the remedy
+	assertContains(t, result.ForLLM, "git_push")
+}
+
+// TestCreatePRTool_DefaultBaseBranch verifies fallback to "main" when BaseBranch is empty.
+func TestCreatePRTool_DefaultBaseBranch(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+
+	// With empty BaseBranch, tool should default to "main"
+	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+		Branch:     "plan/test",
+		BaseBranch: "",
+		Path:       t.TempDir(),
+		RepoRoot:   t.TempDir(),
+	})
+	result := tool.Execute(ctx, map[string]any{
+		"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") {
+		t.Fatal("should not fail on base branch when defaulting to main")
+	}
+}
+
+// TestCreatePRTool_Interface verifies the tool satisfies the Tool interface.
+func TestCreatePRTool_Interface(t *testing.T) {
+	var _ Tool = (*CreatePRTool)(nil)
+
+	tool := NewCreatePRTool(t.TempDir())
+	if tool.Name() != "create_pr" {
+		t.Errorf("Name: got %q, want %q", tool.Name(), "create_pr")
+	}
+	if tool.Description() == "" {
+		t.Error("Description should not be empty")
+	}
+	params := tool.Parameters()
+	if params == nil {
+		t.Fatal("Parameters should not be nil")
+	}
+
+	// Verify "title" is required
+	required, ok := params["required"].([]string)
+	if !ok {
+		t.Fatal("required should be []string")
+	}
+	foundTitle := false
+	for _, r := range required {
+		if r == "title" {
+			foundTitle = true
+		}
+	}
+	if !foundTitle {
+		t.Error("title should be in required parameters")
+	}
+}
+
+// TestAllowedToolsForPreset_GitTools checks git tools are correctly assigned to presets.
+func TestAllowedToolsForPreset_GitTools(t *testing.T) {
+	tests := []struct {
+		name         string
+		preset       Preset
+		wantGitPush  bool
+		wantCreatePR bool
+	}{
+		{"scout", PresetScout, false, false},
+		{"analyst", PresetAnalyst, false, false},
+		{"coder", PresetCoder, true, false},
+		{"worker", PresetWorker, true, true},
+		{"coordinator", PresetCoordinator, true, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			allowed := AllowedToolsForPreset(tt.preset)
+
+			if got := allowed["git_push"]; got != tt.wantGitPush {
+				t.Errorf("git_push: got %v, want %v", got, tt.wantGitPush)
+			}
+			if got := allowed["create_pr"]; got != tt.wantCreatePR {
+				t.Errorf("create_pr: got %v, want %v", got, tt.wantCreatePR)
+			}
+		})
+	}
+}
diff --git a/pkg/tools/gitpush.go b/pkg/tools/gitpush.go
new file mode 100644
index 000000000..0a91dce7f
--- /dev/null
+++ b/pkg/tools/gitpush.go
@@ -0,0 +1,144 @@
+package tools
+
+import (
+	"context"
+	"fmt"
+	"os/exec"
+	"regexp"
+	"strings"
+	"time"
+
+	"github.com/sipeed/picoclaw/pkg/git"
+)
+
+// worktreeInfoKey is the context key for passing WorktreeInfo to tools.
+type worktreeInfoKey struct{}
+
+// WithWorktreeInfo returns a context carrying the active WorktreeInfo.
+func WithWorktreeInfo(ctx context.Context, wt *git.WorktreeInfo) context.Context {
+	return context.WithValue(ctx, worktreeInfoKey{}, wt)
+}
+
+// WorktreeInfoFromCtx extracts the WorktreeInfo from context, or nil.
+func WorktreeInfoFromCtx(ctx context.Context) *git.WorktreeInfo {
+	if v, ok := ctx.Value(worktreeInfoKey{}).(*git.WorktreeInfo); ok {
+		return v
+	}
+	return nil
+}
+
+// protectedBranches are branch names that can never be pushed to.
+var protectedBranches = regexp.MustCompile(`^(main|master|develop|release/.*)$`)
+
+// GitPushTool implements safe git push restricted to worktree branches.
+//
+// Safety invariants:
+//   - Only works inside a worktree (WorktreeInfo must be in context)
+//   - Pushes only the worktree's branch — no arbitrary branch targets
+//   - Protected branches (main, master, develop, release/*) are blocked
+//   - Force push is never allowed
+//   - Auto-commits uncommitted changes before pushing
+type GitPushTool struct {
+	workspace string
+}
+
+// NewGitPushTool creates a GitPushTool.
+func NewGitPushTool(workspace string) *GitPushTool {
+	return &GitPushTool{workspace: workspace}
+}
+
+func (t *GitPushTool) Name() string { return "git_push" }
+
+func (t *GitPushTool) Description() string {
+	return "Push the current worktree branch to origin. Only works inside a git worktree. " +
+		"Auto-commits uncommitted changes before pushing. " +
+		"Protected branches (main, master, develop) cannot be pushed to. Force push is not allowed."
+}
+
+func (t *GitPushTool) Parameters() map[string]any {
+	return map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"commit_message": map[string]any{
+				"type":        "string",
+				"description": "Commit message for uncommitted changes. If omitted, uncommitted changes are auto-committed with a default message.",
+			},
+		},
+		"required": []string{},
+	}
+}
+
+func (t *GitPushTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
+	wt := WorktreeInfoFromCtx(ctx)
+	if wt == nil {
+		return ErrorResult(
+			"git_push requires an active worktree.\n" +
+				"This tool can only be used during worktree-based sessions " +
+				"(e.g., heartbeat tasks or plan executing phase).\n" +
+				"The worktree provides the branch name and isolation boundary — " +
+				"without it, git_push cannot determine which branch to push.")
+	}
+
+	branch := wt.Branch
+	if branch == "" {
+		return ErrorResult(
+			"worktree has no branch name.\n" +
+				"The WorktreeInfo was set but Branch is empty. " +
+				"This is an internal error — the worktree may not have been created correctly.")
+	}
+
+	// Block protected branches
+	if protectedBranches.MatchString(branch) {
+		return ErrorResult(fmt.Sprintf(
+			"cannot push to protected branch %q.\n"+
+				"Protected branches (main, master, develop, release/*) are blocked to prevent "+
+				"accidental overwrites. Work should be done on feature branches created by worktrees.",
+			branch))
+	}
+
+	// Auto-commit uncommitted changes
+	if git.HasUncommittedChanges(wt.Path) {
+		commitMsg := "auto: save before push"
+		if msg, ok := args["commit_message"].(string); ok && msg != "" {
+			commitMsg = msg
+		}
+		if err := git.AutoCommit(wt.Path, commitMsg); err != nil {
+			return ErrorResult(fmt.Sprintf(
+				"auto-commit failed before push: %v\n"+
+					"git_push auto-commits uncommitted changes before pushing. "+
+					"The commit failed, so no push was attempted. "+
+					"Check if the worktree at %q is in a valid state (e.g., no merge conflicts).",
+				err, wt.Path))
+		}
+	}
+
+	// Check there are commits to push
+	ahead := git.CommitsAhead(wt.RepoRoot, wt.BaseBranch, branch)
+	if ahead == 0 {
+		return NewToolResult(fmt.Sprintf(
+			"Nothing to push: branch %q has no commits ahead of %s.\n"+
+				"The branch is identical to the base. Make changes and commit before pushing.",
+			branch, wt.BaseBranch))
+	}
+
+	// Push with -u (set upstream tracking)
+	pushCtx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+	defer cancel()
+
+	cmd := exec.CommandContext(pushCtx, "git", "push", "-u", "origin", branch)
+	cmd.Dir = wt.Path
+	out, err := cmd.CombinedOutput()
+	output := strings.TrimSpace(string(out))
+
+	if err != nil {
+		return ErrorResult(fmt.Sprintf(
+			"git push failed for branch %q: %s\n%s\n"+
+				"Possible causes: network error, authentication failure, or remote rejected the push. "+
+				"If the remote branch has diverged, resolve the divergence in the worktree first — "+
+				"force push is not available.",
+			branch, err, output))
+	}
+
+	return NewToolResult(fmt.Sprintf("Pushed branch %q to origin (%d commit(s) ahead of %s)\n%s",
+		branch, ahead, wt.BaseBranch, output))
+}
diff --git a/pkg/tools/gitpush_test.go b/pkg/tools/gitpush_test.go
new file mode 100644
index 000000000..7842742a2
--- /dev/null
+++ b/pkg/tools/gitpush_test.go
@@ -0,0 +1,184 @@
+package tools
+
+import (
+	"context"
+	"testing"
+
+	"github.com/sipeed/picoclaw/pkg/git"
+)
+
+// TestGitPushTool_NoWorktree verifies that git_push fails without worktree context.
+func TestGitPushTool_NoWorktree(t *testing.T) {
+	tool := NewGitPushTool(t.TempDir())
+
+	result := tool.Execute(context.Background(), map[string]any{})
+	if !result.IsError {
+		t.Fatal("expected error when no worktree in context")
+	}
+	if result.ForLLM == "" {
+		t.Fatal("error message should not be empty")
+	}
+	// Verify helpful guidance is included
+	assertContains(t, result.ForLLM, "worktree")
+	assertContains(t, result.ForLLM, "heartbeat")
+}
+
+// TestGitPushTool_ProtectedBranch verifies that protected branches are blocked.
+func TestGitPushTool_ProtectedBranch(t *testing.T) {
+	tool := NewGitPushTool(t.TempDir())
+
+	protectedNames := []string{"main", "master", "develop", "release/v1.0"}
+
+	for _, branch := range protectedNames {
+		t.Run(branch, func(t *testing.T) {
+			ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+				Branch:     branch,
+				BaseBranch: "main",
+				Path:       t.TempDir(),
+				RepoRoot:   t.TempDir(),
+			})
+			result := tool.Execute(ctx, map[string]any{})
+			if !result.IsError {
+				t.Fatalf("expected error for protected branch %q", branch)
+			}
+			assertContains(t, result.ForLLM, "protected")
+			assertContains(t, result.ForLLM, branch)
+		})
+	}
+}
+
+// TestGitPushTool_EmptyBranch verifies that empty branch name is rejected.
+func TestGitPushTool_EmptyBranch(t *testing.T) {
+	tool := NewGitPushTool(t.TempDir())
+
+	ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+		Branch:     "",
+		BaseBranch: "main",
+		Path:       t.TempDir(),
+		RepoRoot:   t.TempDir(),
+	})
+	result := tool.Execute(ctx, map[string]any{})
+	if !result.IsError {
+		t.Fatal("expected error for empty branch")
+	}
+	assertContains(t, result.ForLLM, "no branch name")
+}
+
+// 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())
+
+	allowedNames := []string{"plan/add-auth", "feature/foo", "worktree/test"}
+
+	for _, branch := range allowedNames {
+		t.Run(branch, func(t *testing.T) {
+			ctx := WithWorktreeInfo(context.Background(), &git.WorktreeInfo{
+				Branch:     branch,
+				BaseBranch: "main",
+				Path:       t.TempDir(),
+				RepoRoot:   t.TempDir(),
+			})
+			result := tool.Execute(ctx, map[string]any{})
+			// Should NOT fail with "protected branch" error
+			if result.IsError && contains(result.ForLLM, "protected") {
+				t.Fatalf("branch %q should not be blocked as protected", branch)
+			}
+		})
+	}
+}
+
+// TestProtectedBranchesRegex tests the regex directly.
+func TestProtectedBranchesRegex(t *testing.T) {
+	tests := []struct {
+		branch    string
+		protected bool
+	}{
+		{"main", true},
+		{"master", true},
+		{"develop", true},
+		{"release/v1.0", true},
+		{"release/2026-03", true},
+		{"plan/add-feature", false},
+		{"feature/main", false}, // "main" not at start
+		{"main-backup", false},  // "main" followed by suffix
+		{"hotfix/urgent", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.branch, func(t *testing.T) {
+			got := protectedBranches.MatchString(tt.branch)
+			if got != tt.protected {
+				t.Errorf("branch %q: got protected=%v, want %v", tt.branch, got, tt.protected)
+			}
+		})
+	}
+}
+
+// TestWorktreeInfoContext verifies context round-trip.
+func TestWorktreeInfoContext(t *testing.T) {
+	wt := &git.WorktreeInfo{
+		Branch:     "plan/test",
+		BaseBranch: "main",
+		Path:       "/tmp/wt",
+		RepoRoot:   "/tmp/repo",
+	}
+
+	ctx := WithWorktreeInfo(context.Background(), wt)
+	got := WorktreeInfoFromCtx(ctx)
+	if got == nil {
+		t.Fatal("expected non-nil WorktreeInfo from context")
+	}
+	if got.Branch != wt.Branch {
+		t.Errorf("Branch: got %q, want %q", got.Branch, wt.Branch)
+	}
+	if got.BaseBranch != wt.BaseBranch {
+		t.Errorf("BaseBranch: got %q, want %q", got.BaseBranch, wt.BaseBranch)
+	}
+
+	// Nil case
+	got2 := WorktreeInfoFromCtx(context.Background())
+	if got2 != nil {
+		t.Errorf("expected nil WorktreeInfo from bare context, got %+v", got2)
+	}
+}
+
+// TestGitPushTool_Interface verifies the tool satisfies the Tool interface.
+func TestGitPushTool_Interface(t *testing.T) {
+	var _ Tool = (*GitPushTool)(nil)
+
+	tool := NewGitPushTool(t.TempDir())
+	if tool.Name() != "git_push" {
+		t.Errorf("Name: got %q, want %q", tool.Name(), "git_push")
+	}
+	if tool.Description() == "" {
+		t.Error("Description should not be empty")
+	}
+	params := tool.Parameters()
+	if params == nil {
+		t.Fatal("Parameters should not be nil")
+	}
+	if params["type"] != "object" {
+		t.Errorf("Parameters type: got %v, want object", params["type"])
+	}
+}
+
+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) {
+		t.Errorf("expected %q to contain %q", s, substr)
+	}
+}
diff --git a/pkg/tools/sandbox.go b/pkg/tools/sandbox.go
index 4a6306393..6e728cf1a 100644
--- a/pkg/tools/sandbox.go
+++ b/pkg/tools/sandbox.go
@@ -108,12 +108,18 @@ func AllowedToolsForPreset(p Preset) map[string]bool {
 		allowed["exec"] = true
 	}
 
-	// Add coder/worker/coordinator tools (write, bg_monitor)
+	// Add coder/worker/coordinator tools (write, bg_monitor, git_push)
 	if p == PresetCoder || p == PresetWorker || p == PresetCoordinator {
 		allowed["write_file"] = true
 		allowed["edit_file"] = true
 		allowed["append_file"] = true
 		allowed["bg_monitor"] = true
+		allowed["git_push"] = true
+	}
+
+	// Add worker/coordinator tools (create_pr)
+	if p == PresetWorker || p == PresetCoordinator {
+		allowed["create_pr"] = true
 	}
 
 	// Add coordinator-only tools (spawn)
diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go
index beecb20b4..1599df454 100644
--- a/pkg/tools/subagent.go
+++ b/pkg/tools/subagent.go
@@ -358,6 +358,14 @@ 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))
+	}
+	if config.AllowedTools["create_pr"] {
+		registry.Register(NewCreatePRTool(sm.workspace))
+	}
+
 	// Register web tools
 	if config.AllowedTools["web_search"] {
 		webSearchTool := NewWebSearchTool(sm.webSearchOpts)

From e279af2a8bcef5d823b8412943e3005113e14e40 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 1 Mar 2026 06:27:09 +0000
Subject: [PATCH 2/4] feat: make create_pr async with background CI polling

create_pr now implements AsyncTool. After successfully creating a PR,
it spawns a background goroutine that polls `gh pr checks` every 30s
(up to 15 min) and reports CI pass/fail via AsyncCallback.

Flow:
1. PR creation returns immediately with AsyncResult (PR URL)
2. Background goroutine waits 10s for CI to register, then polls
3. On pass/fail/no-checks/timeout, calls callback with result
4. Agent receives notification and can act (e.g., gh run view for logs)

https://claude.ai/code/session_01WWttNE5xShanYD6PhMzgKz
---
 pkg/tools/createpr.go      | 132 ++++++++++++++++++++++++++++++++++++-
 pkg/tools/createpr_test.go |  40 ++++++++++-
 2 files changed, 168 insertions(+), 4 deletions(-)

diff --git a/pkg/tools/createpr.go b/pkg/tools/createpr.go
index 165d1f833..4e5b652b6 100644
--- a/pkg/tools/createpr.go
+++ b/pkg/tools/createpr.go
@@ -8,6 +8,11 @@ import (
 	"time"
 )
 
+const (
+	ciPollInterval = 30 * time.Second
+	ciPollTimeout  = 15 * time.Minute
+)
+
 // CreatePRTool creates a GitHub pull request from the current worktree branch.
 //
 // Safety invariants:
@@ -16,8 +21,14 @@ import (
 //   - Requires the branch to be already pushed (use git_push first)
 //   - Checks for merge conflicts with base before creating
 //   - Uses `gh pr create` under the hood
+//
+// Async behavior:
+//   - PR creation itself is synchronous and returns immediately with the PR URL
+//   - 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.
@@ -27,11 +38,17 @@ func NewCreatePRTool(workspace string) *CreatePRTool {
 
 func (t *CreatePRTool) Name() string { return "create_pr" }
 
+// SetCallback implements AsyncTool for CI completion notification.
+func (t *CreatePRTool) SetCallback(cb AsyncCallback) {
+	t.callback = cb
+}
+
 func (t *CreatePRTool) Description() string {
 	return "Create a GitHub pull request from the current worktree branch. " +
 		"The base branch is auto-detected from the worktree's parent branch. " +
 		"The branch must be pushed to origin first (use git_push). " +
 		"Checks for merge conflicts with the base branch before creating. " +
+		"After PR creation, polls CI status in the background and notifies when complete. " +
 		"Requires the `gh` CLI to be installed and authenticated."
 }
 
@@ -168,8 +185,117 @@ func (t *CreatePRTool) Execute(ctx context.Context, args map[string]any) *ToolRe
 			err, output, branch))
 	}
 
-	return NewToolResult(fmt.Sprintf(
+	prURL := output // gh pr create outputs the PR URL
+
+	// Start background CI polling if callback is set
+	if t.callback != nil && prURL != "" {
+		cb := t.callback
+		repoRoot := wt.RepoRoot
+		go pollCIStatus(repoRoot, prURL, cb)
+	}
+
+	return AsyncResult(fmt.Sprintf(
 		"Pull request created: %s\n"+
-			"Branch: %s -> %s",
-		output, branch, baseBranch))
+			"Branch: %s -> %s\n"+
+			"CI status will be reported asynchronously when checks complete.",
+		prURL, branch, baseBranch))
+}
+
+// pollCIStatus polls `gh pr checks` in the background until all checks
+// pass, fail, or the timeout is reached. Reports back via AsyncCallback.
+func pollCIStatus(repoRoot, prURL string, callback AsyncCallback) {
+	// Detached context with hard timeout — this goroutine outlives the tool call.
+	ctx, cancel := context.WithTimeout(context.Background(), ciPollTimeout)
+	defer cancel()
+
+	// Initial wait: CI runs take a few seconds to register after PR creation
+	select {
+	case <-time.After(10 * time.Second):
+	case <-ctx.Done():
+		return
+	}
+
+	ticker := time.NewTicker(ciPollInterval)
+	defer ticker.Stop()
+
+	for {
+		status, detail := checkPRChecks(ctx, repoRoot, prURL)
+		switch status {
+		case ciStatusPass:
+			callback(ctx, NewToolResult(fmt.Sprintf(
+				"CI passed for %s\n%s",
+				prURL, detail)))
+			return
+		case ciStatusFail:
+			callback(ctx, &ToolResult{
+				ForLLM: fmt.Sprintf(
+					"CI failed for %s\n%s\n"+
+						"Run `gh run view` for detailed logs.",
+					prURL, detail),
+				IsError: true,
+			})
+			return
+		case ciStatusNone:
+			callback(ctx, NewToolResult(fmt.Sprintf(
+				"No CI checks configured for %s. PR is ready for review.",
+				prURL)))
+			return
+		case ciStatusPending:
+			// Still running, continue polling
+		}
+
+		select {
+		case <-ticker.C:
+		case <-ctx.Done():
+			callback(ctx, &ToolResult{
+				ForLLM: fmt.Sprintf(
+					"CI polling timed out after %s for %s.\n"+
+						"Checks may still be running. Run `gh pr checks %s` to check.",
+					ciPollTimeout, prURL, prURL),
+				IsError: true,
+			})
+			return
+		}
+	}
+}
+
+type ciStatus int
+
+const (
+	ciStatusPending ciStatus = iota
+	ciStatusPass
+	ciStatusFail
+	ciStatusNone
+)
+
+// checkPRChecks runs `gh pr checks` and parses the result.
+// Returns the aggregate status and raw output for the caller to include.
+func checkPRChecks(ctx context.Context, repoRoot, prURL string) (ciStatus, string) {
+	checkCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
+	defer cancel()
+
+	cmd := exec.CommandContext(checkCtx, "gh", "pr", "checks", prURL)
+	cmd.Dir = repoRoot
+	out, err := cmd.CombinedOutput()
+	output := strings.TrimSpace(string(out))
+
+	if err != nil {
+		// gh pr checks exits 1 when any check has failed
+		if strings.Contains(output, "fail") || strings.Contains(output, "X ") {
+			return ciStatusFail, output
+		}
+		// "no checks" case
+		if strings.Contains(output, "no checks") || output == "" {
+			return ciStatusNone, ""
+		}
+		// Transient error or still pending — keep polling
+		return ciStatusPending, output
+	}
+
+	// Exit 0: all checks completed. Check for pending.
+	if strings.Contains(output, "pending") || strings.Contains(output, "- ") {
+		return ciStatusPending, output
+	}
+
+	return ciStatusPass, output
 }
diff --git a/pkg/tools/createpr_test.go b/pkg/tools/createpr_test.go
index e9392ef88..0100d6171 100644
--- a/pkg/tools/createpr_test.go
+++ b/pkg/tools/createpr_test.go
@@ -111,9 +111,10 @@ func TestCreatePRTool_DefaultBaseBranch(t *testing.T) {
 	}
 }
 
-// TestCreatePRTool_Interface verifies the tool satisfies the Tool interface.
+// TestCreatePRTool_Interface verifies the tool satisfies both Tool and AsyncTool interfaces.
 func TestCreatePRTool_Interface(t *testing.T) {
 	var _ Tool = (*CreatePRTool)(nil)
+	var _ AsyncTool = (*CreatePRTool)(nil)
 
 	tool := NewCreatePRTool(t.TempDir())
 	if tool.Name() != "create_pr" {
@@ -143,6 +144,43 @@ func TestCreatePRTool_Interface(t *testing.T) {
 	}
 }
 
+// TestCreatePRTool_SetCallback verifies callback is stored.
+func TestCreatePRTool_SetCallback(t *testing.T) {
+	tool := NewCreatePRTool(t.TempDir())
+	if tool.callback != nil {
+		t.Fatal("callback should be nil initially")
+	}
+
+	called := false
+	tool.SetCallback(func(ctx context.Context, result *ToolResult) {
+		called = true
+	})
+	if tool.callback == nil {
+		t.Fatal("callback should be set after SetCallback")
+	}
+	// Verify it's callable (doesn't panic)
+	tool.callback(context.Background(), NewToolResult("test"))
+	if !called {
+		t.Fatal("callback was not invoked")
+	}
+}
+
+// TestCheckPRChecks_ParseResults tests CI status parsing logic.
+func TestCheckPRChecks_ParseResults(t *testing.T) {
+	// This tests the parsing logic conceptually — actual `gh` calls
+	// would need integration tests. We verify the status constants exist
+	// and the type is usable.
+	if ciStatusPending != 0 {
+		t.Error("ciStatusPending should be 0 (default)")
+	}
+	if ciStatusPass == ciStatusFail {
+		t.Error("ciStatusPass and ciStatusFail should differ")
+	}
+	if ciStatusNone == ciStatusPending {
+		t.Error("ciStatusNone and ciStatusPending should differ")
+	}
+}
+
 // TestAllowedToolsForPreset_GitTools checks git tools are correctly assigned to presets.
 func TestAllowedToolsForPreset_GitTools(t *testing.T) {
 	tests := []struct {

From 704e8cd16d9462fb27b0bd7e3517e6b386ae2843 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 1 Mar 2026 06:45:18 +0000
Subject: [PATCH 3/4] fix: wire asyncCallback to PublishInbound for CI result
 delivery

The asyncCallback in runLLMIteration was a no-op that only logged.
For spawn this didn't matter (SubagentManager.runTask does its own
PublishInbound), but create_pr's CI goroutine had no way to deliver
results back to the conductor.

Now asyncCallback publishes a system inbound message with
senderID="async:<tool>" so processSystemMessage injects it into
the conductor's session history. The conductor sees the CI result
on its next turn.

https://claude.ai/code/session_01WWttNE5xShanYD6PhMzgKz
---
 pkg/agent/loop.go | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go
index 2417597a3..32e2584a8 100644
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -2614,20 +2614,35 @@ func (al *AgentLoop) runLLMIteration(
 				}
 			}
 
-			// Create async callback for tools that implement AsyncTool
-			// NOTE: Following openclaw's design, async tools do NOT send results directly to users.
-			// Instead, they notify the agent via PublishInbound, and the agent decides
-			// whether to forward the result to the user (in processSystemMessage).
+			// Create async callback for tools that implement AsyncTool.
+			// The callback publishes a system inbound message so processSystemMessage
+			// injects the result into the conductor's session history. The conductor
+			// sees it on its next turn and decides whether to notify the user.
+			toolName := tc.Name // capture for goroutine
 			asyncCallback := func(callbackCtx context.Context, result *tools.ToolResult) {
-				// Log the async completion but don't send directly to user
-				// The agent will handle user notification via processSystemMessage
-				if !result.Silent && result.ForUser != "" {
-					logger.InfoCF("agent", "Async tool completed, agent will handle notification",
-						map[string]any{
-							"tool":        tc.Name,
-							"content_len": len(result.ForUser),
-						})
+				content := result.ForLLM
+				if content == "" {
+					content = result.ForUser
 				}
+				if content == "" {
+					return
+				}
+
+				logger.InfoCF("agent", "Async tool completed, publishing to conductor",
+					map[string]any{
+						"tool":        toolName,
+						"content_len": len(content),
+						"is_error":    result.IsError,
+					})
+
+				pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second)
+				defer pubCancel()
+				_ = al.bus.PublishInbound(pubCtx, bus.InboundMessage{
+					Channel:  "system",
+					SenderID: fmt.Sprintf("async:%s", toolName),
+					ChatID:   fmt.Sprintf("%s:%s", opts.Channel, opts.ChatID),
+					Content:  fmt.Sprintf("Async tool '%s' completed.\n\nResult:\n%s", toolName, content),
+				})
 			}
 
 			// Report toolcall state to canvas.

From a056918d6a454129cff59a59e029dcb6012035db Mon Sep 17 00:00:00 2001
From: dj-oyu <68707227+dj-oyu@users.noreply.github.com>
Date: Mon, 2 Mar 2026 01:46:22 +0900
Subject: [PATCH 4/4] 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      |  4 ++--
 pkg/tools/createpr.go      | 10 +++++-----
 pkg/tools/createpr_test.go | 17 +++++++++--------
 pkg/tools/gitpush.go       |  8 +++-----
 pkg/tools/gitpush_test.go  | 28 ++++++++--------------------
 pkg/tools/subagent.go      |  4 ++--
 6 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go
index a52024dd2..e0cf3ebf1 100644
--- a/pkg/agent/instance.go
+++ b/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)
diff --git a/pkg/tools/createpr.go b/pkg/tools/createpr.go
index 4e5b652b6..c0f351fb3 100644
--- a/pkg/tools/createpr.go
+++ b/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
+	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,
diff --git a/pkg/tools/createpr_test.go b/pkg/tools/createpr_test.go
index 0100d6171..26c330b7c 100644
--- a/pkg/tools/createpr_test.go
+++ b/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")
 	}
diff --git a/pkg/tools/gitpush.go b/pkg/tools/gitpush.go
index 0a91dce7f..3d8bc2b9b 100644
--- a/pkg/tools/gitpush.go
+++ b/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" }
diff --git a/pkg/tools/gitpush_test.go b/pkg/tools/gitpush_test.go
index 7842742a2..d16c446d4 100644
--- a/pkg/tools/gitpush_test.go
+++ b/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)
 	}
 }
diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go
index 1599df454..157b40d4b 100644
--- a/pkg/tools/subagent.go
+++ b/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