Merge pull request #11 from dj-oyu/feat/git-push-create-pr
feat: add git_push and create_pr tools with async CI polling
This commit is contained in:
commit
54dfec1347
8 changed files with 872 additions and 13 deletions
|
|
@ -80,6 +80,8 @@ func NewAgentInstance(
|
||||||
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
|
toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict))
|
||||||
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
|
toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
|
||||||
toolsRegistry.Register(tools.NewLogsTool())
|
toolsRegistry.Register(tools.NewLogsTool())
|
||||||
|
toolsRegistry.Register(tools.NewGitPushTool())
|
||||||
|
toolsRegistry.Register(tools.NewCreatePRTool())
|
||||||
|
|
||||||
sessionsDir := filepath.Join(workspace, "sessions")
|
sessionsDir := filepath.Join(workspace, "sessions")
|
||||||
sessionsManager := session.NewSessionManager(sessionsDir)
|
sessionsManager := session.NewSessionManager(sessionsDir)
|
||||||
|
|
|
||||||
|
|
@ -2614,20 +2614,35 @@ func (al *AgentLoop) runLLMIteration(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create async callback for tools that implement AsyncTool
|
// Create async callback for tools that implement AsyncTool.
|
||||||
// NOTE: Following openclaw's design, async tools do NOT send results directly to users.
|
// The callback publishes a system inbound message so processSystemMessage
|
||||||
// Instead, they notify the agent via PublishInbound, and the agent decides
|
// injects the result into the conductor's session history. The conductor
|
||||||
// whether to forward the result to the user (in processSystemMessage).
|
// 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) {
|
asyncCallback := func(callbackCtx context.Context, result *tools.ToolResult) {
|
||||||
// Log the async completion but don't send directly to user
|
content := result.ForLLM
|
||||||
// The agent will handle user notification via processSystemMessage
|
if content == "" {
|
||||||
if !result.Silent && result.ForUser != "" {
|
content = result.ForUser
|
||||||
logger.InfoCF("agent", "Async tool completed, agent will handle notification",
|
|
||||||
map[string]any{
|
|
||||||
"tool": tc.Name,
|
|
||||||
"content_len": len(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.
|
// Report toolcall state to canvas.
|
||||||
|
|
@ -2637,6 +2652,7 @@ func (al *AgentLoop) runLLMIteration(
|
||||||
toolCtx := ctx
|
toolCtx := ctx
|
||||||
if wt := agent.GetWorktree(opts.SessionKey); wt != nil {
|
if wt := agent.GetWorktree(opts.SessionKey); wt != nil {
|
||||||
toolCtx = tools.WithWorkspaceOverride(toolCtx, wt.Path)
|
toolCtx = tools.WithWorkspaceOverride(toolCtx, wt.Path)
|
||||||
|
toolCtx = tools.WithWorktreeInfo(toolCtx, wt)
|
||||||
}
|
}
|
||||||
toolResult := agent.Tools.ExecuteWithContext(
|
toolResult := agent.Tools.ExecuteWithContext(
|
||||||
toolCtx,
|
toolCtx,
|
||||||
|
|
|
||||||
301
pkg/tools/createpr.go
Normal file
301
pkg/tools/createpr.go
Normal file
|
|
@ -0,0 +1,301 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"os/exec"
|
||||||
|
"strings"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
ciPollInterval = 30 * time.Second
|
||||||
|
ciPollTimeout = 15 * time.Minute
|
||||||
|
)
|
||||||
|
|
||||||
|
// 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
|
||||||
|
//
|
||||||
|
// 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 {
|
||||||
|
callback AsyncCallback
|
||||||
|
}
|
||||||
|
|
||||||
|
// NewCreatePRTool creates a CreatePRTool.
|
||||||
|
func NewCreatePRTool() *CreatePRTool {
|
||||||
|
return &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."
|
||||||
|
}
|
||||||
|
|
||||||
|
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))
|
||||||
|
}
|
||||||
|
|
||||||
|
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\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
|
||||||
|
}
|
||||||
212
pkg/tools/createpr_test.go
Normal file
212
pkg/tools/createpr_test.go
Normal file
|
|
@ -0,0 +1,212 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
// 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 && strings.Contains(result.ForLLM, "base branch") {
|
||||||
|
t.Fatal("should not fail on base branch when defaulting to main")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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()
|
||||||
|
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")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCreatePRTool_SetCallback verifies callback is stored.
|
||||||
|
func TestCreatePRTool_SetCallback(t *testing.T) {
|
||||||
|
tool := NewCreatePRTool()
|
||||||
|
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 {
|
||||||
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
142
pkg/tools/gitpush.go
Normal file
142
pkg/tools/gitpush.go
Normal file
|
|
@ -0,0 +1,142 @@
|
||||||
|
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{}
|
||||||
|
|
||||||
|
// NewGitPushTool creates a GitPushTool.
|
||||||
|
func NewGitPushTool() *GitPushTool {
|
||||||
|
return &GitPushTool{}
|
||||||
|
}
|
||||||
|
|
||||||
|
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))
|
||||||
|
}
|
||||||
172
pkg/tools/gitpush_test.go
Normal file
172
pkg/tools/gitpush_test.go
Normal file
|
|
@ -0,0 +1,172 @@
|
||||||
|
package tools
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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()
|
||||||
|
|
||||||
|
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 && strings.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()
|
||||||
|
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 assertContains(t *testing.T, s, substr string) {
|
||||||
|
t.Helper()
|
||||||
|
if !strings.Contains(s, substr) {
|
||||||
|
t.Errorf("expected %q to contain %q", s, substr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -108,12 +108,18 @@ func AllowedToolsForPreset(p Preset) map[string]bool {
|
||||||
allowed["exec"] = true
|
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 {
|
if p == PresetCoder || p == PresetWorker || p == PresetCoordinator {
|
||||||
allowed["write_file"] = true
|
allowed["write_file"] = true
|
||||||
allowed["edit_file"] = true
|
allowed["edit_file"] = true
|
||||||
allowed["append_file"] = true
|
allowed["append_file"] = true
|
||||||
allowed["bg_monitor"] = 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)
|
// Add coordinator-only tools (spawn)
|
||||||
|
|
|
||||||
|
|
@ -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())
|
||||||
|
}
|
||||||
|
if config.AllowedTools["create_pr"] {
|
||||||
|
registry.Register(NewCreatePRTool())
|
||||||
|
}
|
||||||
|
|
||||||
// Register web tools
|
// Register web tools
|
||||||
if config.AllowedTools["web_search"] {
|
if config.AllowedTools["web_search"] {
|
||||||
webSearchTool := NewWebSearchTool(sm.webSearchOpts)
|
webSearchTool := NewWebSearchTool(sm.webSearchOpts)
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue