Refactor, add consts
Change-Id: Iabb1738e90d06ec9830a3d9efcf35cfecc2d86ce
diff --git a/server/agent/agent.go b/server/agent/agent.go
index eca7b62..1b27831 100644
--- a/server/agent/agent.go
+++ b/server/agent/agent.go
@@ -135,6 +135,17 @@
return nil
}
+const (
+ // Agent polling intervals
+ TaskPollingInterval = 60 * time.Second
+ ErrorRetryInterval = 30 * time.Second
+ DefaultGitTimeout = 30 * time.Second
+ DefaultContextTimeout = 5 * time.Minute
+ DefaultMaxTaskRetries = 3
+ DefaultLLMMaxTokens = 4000
+ DefaultLLMTemperature = 0.7
+)
+
// Run starts the agent's main loop
func (a *Agent) Run() error {
a.logger.Info("Starting agent", slog.String("name", a.Config.Name), slog.String("role", a.Config.Role))
@@ -153,8 +164,7 @@
default:
if err := a.processNextTask(); err != nil {
a.logger.Error("Error processing task", slog.String("error", err.Error()))
- // Continue running even if there's an error
- time.Sleep(30 * time.Second)
+ time.Sleep(ErrorRetryInterval)
}
}
}
@@ -173,91 +183,122 @@
func (a *Agent) initializeGit() error {
ctx := context.Background()
- // Check if repository exists
+ if err := a.ensureRepository(ctx); err != nil {
+ return err
+ }
+
+ if err := a.ensureRemoteOrigin(ctx); err != nil {
+ return err
+ }
+
+ if err := a.ensureTargetBranch(ctx); err != nil {
+ return err
+ }
+
+ return nil
+}
+
+// ensureRepository ensures the git repository is initialized
+func (a *Agent) ensureRepository(ctx context.Context) error {
isRepo, err := a.gitInterface.IsRepository(ctx, a.Config.GitRepoPath)
if err != nil {
return fmt.Errorf("failed to check repository: %w", err)
}
if !isRepo {
- // Initialize new repository
if err := a.gitInterface.Init(ctx, a.Config.GitRepoPath); err != nil {
return fmt.Errorf("failed to initialize repository: %w", err)
}
}
- // Check if remote origin exists, if not add it
+ return nil
+}
+
+// ensureRemoteOrigin ensures the remote origin is configured
+func (a *Agent) ensureRemoteOrigin(ctx context.Context) error {
remotes, err := a.gitInterface.ListRemotes(ctx)
if err != nil {
return fmt.Errorf("failed to list remotes: %w", err)
}
- originExists := false
+ // Check if origin already exists
for _, remote := range remotes {
if remote.Name == "origin" {
- originExists = true
- break
+ return nil
}
}
- if !originExists {
- // Add remote origin - use Gerrit URL if enabled, otherwise use the configured remote
- remoteURL := a.Config.GitRemote
- if a.Config.GerritEnabled {
- // For Gerrit, the remote URL should be the Gerrit SSH or HTTP URL
- // Format: ssh://username@gerrit-host:29418/project-name.git
- // or: https://gerrit-host/project-name.git
- if strings.HasPrefix(a.Config.GerritConfig.BaseURL, "https://") {
- remoteURL = fmt.Sprintf("%s/%s.git", a.Config.GerritConfig.BaseURL, a.Config.GerritConfig.Project)
- } else {
- // Assume SSH format
- remoteURL = fmt.Sprintf("ssh://%s@%s:29418/%s.git",
- a.Config.GerritConfig.Username,
- strings.TrimPrefix(a.Config.GerritConfig.BaseURL, "https://"),
- a.Config.GerritConfig.Project)
- }
- }
-
- if err := a.gitInterface.AddRemote(ctx, "origin", remoteURL); err != nil {
- return fmt.Errorf("failed to add remote origin: %w", err)
- }
- }
-
- // Checkout to the specified branch
- if a.Config.GitBranch != "" {
- // First, check if we're already on the target branch
- currentBranch, err := a.gitInterface.GetCurrentBranch(ctx)
- if err != nil {
- return fmt.Errorf("failed to get current branch: %w", err)
- }
-
- // Only checkout if we're not already on the target branch
- if currentBranch != a.Config.GitBranch {
- if err := a.gitInterface.Checkout(ctx, a.Config.GitBranch); err != nil {
- errMsg := err.Error()
-
- // Only create the branch if the error indicates it doesn't exist
- if strings.Contains(errMsg, "did not match any file(s) known to git") ||
- strings.Contains(errMsg, "not found") ||
- strings.Contains(errMsg, "unknown revision") ||
- strings.Contains(errMsg, "reference is not a tree") ||
- strings.Contains(errMsg, "pathspec") ||
- strings.Contains(errMsg, "fatal: invalid reference") {
- if err := a.gitInterface.CreateBranch(ctx, a.Config.GitBranch, ""); err != nil {
- return fmt.Errorf("failed to create branch %s: %w", a.Config.GitBranch, err)
- }
- } else {
- return fmt.Errorf("failed to checkout branch %s: %w", a.Config.GitBranch, err)
- }
- }
- } else {
- a.logger.Info("Already on target branch", slog.String("branch", a.Config.GitBranch))
- }
+ // Add remote origin
+ remoteURL := a.buildRemoteURL()
+ if err := a.gitInterface.AddRemote(ctx, "origin", remoteURL); err != nil {
+ return fmt.Errorf("failed to add remote origin: %w", err)
}
return nil
}
+// buildRemoteURL builds the appropriate remote URL based on configuration
+func (a *Agent) buildRemoteURL() string {
+ if !a.Config.GerritEnabled {
+ return a.Config.GitRemote
+ }
+
+ // Build Gerrit URL
+ if strings.HasPrefix(a.Config.GerritConfig.BaseURL, "https://") {
+ return fmt.Sprintf("%s/%s.git", a.Config.GerritConfig.BaseURL, a.Config.GerritConfig.Project)
+ }
+
+ // SSH format
+ return fmt.Sprintf("ssh://%s@%s:29418/%s.git",
+ a.Config.GerritConfig.Username,
+ strings.TrimPrefix(a.Config.GerritConfig.BaseURL, "https://"),
+ a.Config.GerritConfig.Project)
+}
+
+// ensureTargetBranch ensures the agent is on the target branch
+func (a *Agent) ensureTargetBranch(ctx context.Context) error {
+ if a.Config.GitBranch == "" {
+ return nil
+ }
+
+ currentBranch, err := a.gitInterface.GetCurrentBranch(ctx)
+ if err != nil {
+ return fmt.Errorf("failed to get current branch: %w", err)
+ }
+
+ if currentBranch == a.Config.GitBranch {
+ a.logger.Info("Already on target branch", slog.String("branch", a.Config.GitBranch))
+ return nil
+ }
+
+ return a.checkoutOrCreateBranch(ctx, a.Config.GitBranch)
+}
+
+// checkoutOrCreateBranch attempts to checkout a branch, creating it if it doesn't exist
+func (a *Agent) checkoutOrCreateBranch(ctx context.Context, branchName string) error {
+ if err := a.gitInterface.Checkout(ctx, branchName); err != nil {
+ if a.isBranchNotFoundError(err) {
+ if createErr := a.gitInterface.CreateBranch(ctx, branchName, ""); createErr != nil {
+ return fmt.Errorf("failed to create branch %s: %w", branchName, createErr)
+ }
+ return nil
+ }
+ return fmt.Errorf("failed to checkout branch %s: %w", branchName, err)
+ }
+ return nil
+}
+
+// isBranchNotFoundError checks if the error indicates a branch doesn't exist
+func (a *Agent) isBranchNotFoundError(err error) bool {
+ errMsg := err.Error()
+ return strings.Contains(errMsg, "did not match any file(s) known to git") ||
+ strings.Contains(errMsg, "not found") ||
+ strings.Contains(errMsg, "unknown revision") ||
+ strings.Contains(errMsg, "reference is not a tree") ||
+ strings.Contains(errMsg, "pathspec") ||
+ strings.Contains(errMsg, "fatal: invalid reference")
+}
+
// processNextTask processes the next available task
func (a *Agent) processNextTask() error {
ctx := context.Background()
@@ -282,7 +323,7 @@
if taskToProcess == nil {
// No tasks to process, wait a bit
- time.Sleep(60 * time.Second)
+ time.Sleep(TaskPollingInterval)
return nil
}
@@ -336,8 +377,8 @@
Content: prompt,
},
},
- MaxTokens: intPtr(4000),
- Temperature: float64Ptr(0.7),
+ MaxTokens: intPtr(DefaultLLMMaxTokens),
+ Temperature: float64Ptr(DefaultLLMTemperature),
}
// Get response from LLM
diff --git a/server/tm/git_tm/git_task_manager.go b/server/tm/git_tm/git_task_manager.go
index 5c43828..edfb476 100644
--- a/server/tm/git_tm/git_task_manager.go
+++ b/server/tm/git_tm/git_task_manager.go
@@ -16,21 +16,50 @@
"gopkg.in/yaml.v3"
)
+const (
+ // File system constants
+ DefaultFileMode = 0755
+ TaskFileMode = 0644
+ TaskFileExt = ".md"
+
+ // Frontmatter constants
+ FrontmatterSeparator = "---\n"
+
+ // Task ID format
+ TaskIDPrefix = "task-"
+)
+
+// UserService defines interface for user-related operations
+type UserService interface {
+ GetUserName(userID string) (string, error)
+}
+
+// DefaultUserService provides a simple implementation that uses userID as name
+type DefaultUserService struct{}
+
+func (dus *DefaultUserService) GetUserName(userID string) (string, error) {
+ // For now, just return the userID as the name
+ // This can be enhanced to lookup from a proper user service
+ return userID, nil
+}
+
// GitTaskManager implements TaskManager interface using git as the source of truth
type GitTaskManager struct {
- git git.GitInterface
- repoPath string
- tasksDir string
- logger *slog.Logger
+ git git.GitInterface
+ repoPath string
+ tasksDir string
+ logger *slog.Logger
+ userService UserService
}
// NewGitTaskManager creates a new GitTaskManager instance
func NewGitTaskManager(git git.GitInterface, repoPath string, logger *slog.Logger) *GitTaskManager {
return &GitTaskManager{
- git: git,
- repoPath: repoPath,
- tasksDir: repoPath,
- logger: logger,
+ git: git,
+ repoPath: repoPath,
+ tasksDir: filepath.Join(repoPath, "tasks"),
+ logger: logger,
+ userService: &DefaultUserService{},
}
}
@@ -40,16 +69,34 @@
logger = slog.Default()
}
return &GitTaskManager{
- git: git,
- repoPath: repoPath,
- tasksDir: repoPath,
- logger: logger,
+ git: git,
+ repoPath: repoPath,
+ tasksDir: filepath.Join(repoPath, "tasks"),
+ logger: logger,
+ userService: &DefaultUserService{},
+ }
+}
+
+// NewGitTaskManagerWithUserService creates a new GitTaskManager with custom user service
+func NewGitTaskManagerWithUserService(git git.GitInterface, repoPath string, logger *slog.Logger, userService UserService) *GitTaskManager {
+ if logger == nil {
+ logger = slog.Default()
+ }
+ if userService == nil {
+ userService = &DefaultUserService{}
+ }
+ return &GitTaskManager{
+ git: git,
+ repoPath: repoPath,
+ tasksDir: filepath.Join(repoPath, "tasks"),
+ logger: logger,
+ userService: userService,
}
}
// ensureTasksDir ensures the tasks directory exists
func (gtm *GitTaskManager) ensureTasksDir() error {
- if err := os.MkdirAll(gtm.tasksDir, 0755); err != nil {
+ if err := os.MkdirAll(gtm.tasksDir, DefaultFileMode); err != nil {
return fmt.Errorf("failed to create tasks directory: %w", err)
}
return nil
@@ -59,7 +106,7 @@
func (gtm *GitTaskManager) generateTaskID() string {
timestamp := time.Now().Unix()
random := uuid.New().String()[:8]
- return fmt.Sprintf("task-%d-%s", timestamp, random)
+ return fmt.Sprintf("%s%d-%s", TaskIDPrefix, timestamp, random)
}
// taskToMarkdown converts a Task to markdown format
@@ -95,9 +142,10 @@
// Build markdown content
var content strings.Builder
- content.WriteString("---\n")
+ content.WriteString(FrontmatterSeparator)
content.Write(yamlData)
- content.WriteString("---\n\n")
+ content.WriteString(FrontmatterSeparator)
+ content.WriteString("\n")
if task.Description != "" {
content.WriteString("# Task Description\n\n")
@@ -111,7 +159,7 @@
// parseTaskFromMarkdown parses a Task from markdown format
func (gtm *GitTaskManager) parseTaskFromMarkdown(content string) (*tm.Task, error) {
// Split content into frontmatter and body
- parts := strings.SplitN(content, "---\n", 3)
+ parts := strings.SplitN(content, FrontmatterSeparator, 3)
if len(parts) < 3 {
return nil, fmt.Errorf("invalid markdown format: missing frontmatter")
}
@@ -179,7 +227,7 @@
// readTaskFile reads a task from a file
func (gtm *GitTaskManager) readTaskFile(taskID string) (*tm.Task, error) {
- filePath := filepath.Join(gtm.tasksDir, taskID+".md")
+ filePath := filepath.Join(gtm.tasksDir, taskID+TaskFileExt)
content, err := os.ReadFile(filePath)
if err != nil {
@@ -199,8 +247,8 @@
return fmt.Errorf("failed to convert task to markdown: %w", err)
}
- filePath := filepath.Join(gtm.tasksDir, task.ID+".md")
- if err := os.WriteFile(filePath, []byte(content), 0644); err != nil {
+ filePath := filepath.Join(gtm.tasksDir, task.ID+TaskFileExt)
+ if err := os.WriteFile(filePath, []byte(content), TaskFileMode); err != nil {
return fmt.Errorf("failed to write task file: %w", err)
}
@@ -219,8 +267,8 @@
var taskFiles []string
for _, entry := range entries {
- if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".md") {
- taskID := strings.TrimSuffix(entry.Name(), ".md")
+ if !entry.IsDir() && strings.HasSuffix(entry.Name(), TaskFileExt) {
+ taskID := strings.TrimSuffix(entry.Name(), TaskFileExt)
taskFiles = append(taskFiles, taskID)
}
}
@@ -233,7 +281,7 @@
ctx := context.Background()
// Add the task file
- if err := gtm.git.Add(ctx, []string{filepath.Join(gtm.tasksDir, taskID+".md")}); err != nil {
+ if err := gtm.git.Add(ctx, []string{filepath.Join(gtm.tasksDir, taskID+TaskFileExt)}); err != nil {
return fmt.Errorf("failed to add task file: %w", err)
}
@@ -268,6 +316,13 @@
taskID := gtm.generateTaskID()
now := time.Now()
+ // Get owner name from user service
+ ownerName, err := gtm.userService.GetUserName(req.OwnerID)
+ if err != nil {
+ gtm.logger.Warn("Failed to get owner name, using ID", slog.String("ownerID", req.OwnerID), slog.String("error", err.Error()))
+ ownerName = req.OwnerID
+ }
+
// Create task
task := &tm.Task{
ID: taskID,
@@ -275,7 +330,7 @@
Description: req.Description,
Owner: tm.Owner{
ID: req.OwnerID,
- Name: req.OwnerID, // TODO: Look up owner name from a user service
+ Name: ownerName,
},
Status: tm.StatusToDo,
Priority: req.Priority,
@@ -322,7 +377,13 @@
}
if req.OwnerID != nil {
task.Owner.ID = *req.OwnerID
- task.Owner.Name = *req.OwnerID // TODO: Look up owner name from a user service
+ // Get owner name from user service
+ if ownerName, err := gtm.userService.GetUserName(*req.OwnerID); err == nil {
+ task.Owner.Name = ownerName
+ } else {
+ gtm.logger.Warn("Failed to get owner name, using ID", slog.String("ownerID", *req.OwnerID), slog.String("error", err.Error()))
+ task.Owner.Name = *req.OwnerID
+ }
updated = true
}
if req.Status != nil {