From 67249fa78f8142abb2b33a3bebf2f8966c17feaa Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 21 Mar 2026 16:22:18 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20address=20Codex=20round=203=20findings?= =?UTF-8?q?=20=E2=80=94=205=20high,=204=20medium,=201=20low?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit High: prep creates workspace dir before clone (was missing) High: auto_pr detects default branch instead of hardcoding main High: mirror gh pr commands now use --repo for correct targeting High: syncRepos HTTP client has 15s timeout (was no timeout) High: sync timestamp only advances when all repos were pulled Medium: rebaseBranch uses detected default branch Medium: scan URL-encodes labels to prevent injection Medium: recall MinConfidence forwarding (acknowledged, API-level) Medium: recall tags preservation (acknowledged, API-level) Low: harvest pushBranch uses coreerr.E instead of fmt.Errorf Shared gitDefaultBranch helper added to agentic/paths.go. Co-Authored-By: Virgil --- pkg/agentic/auto_pr.go | 9 ++++++--- pkg/agentic/mirror.go | 4 +++- pkg/agentic/paths.go | 22 ++++++++++++++++++++++ pkg/agentic/prep.go | 5 +++++ pkg/agentic/scan.go | 12 +++++++++--- pkg/agentic/verify.go | 12 +++++++----- pkg/monitor/harvest.go | 3 ++- pkg/monitor/sync.go | 17 +++++++++++------ 8 files changed, 65 insertions(+), 19 deletions(-) diff --git a/pkg/agentic/auto_pr.go b/pkg/agentic/auto_pr.go index 2771bfd..124b05d 100644 --- a/pkg/agentic/auto_pr.go +++ b/pkg/agentic/auto_pr.go @@ -21,8 +21,11 @@ func (s *PrepSubsystem) autoCreatePR(wsDir string) { srcDir := filepath.Join(wsDir, "src") - // Check if there are commits on the branch beyond origin/main - diffCmd := exec.Command("git", "log", "--oneline", "origin/main..HEAD") + // Detect default branch for this repo + base := gitDefaultBranch(srcDir) + + // Check if there are commits on the branch beyond the default branch + diffCmd := exec.Command("git", "log", "--oneline", "origin/"+base+"..HEAD") diffCmd.Dir = srcDir out, err := diffCmd.Output() if err != nil || len(strings.TrimSpace(string(out))) == 0 { @@ -58,7 +61,7 @@ func (s *PrepSubsystem) autoCreatePR(wsDir string) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - prURL, _, err := s.forgeCreatePR(ctx, org, st.Repo, st.Branch, "main", title, body) + prURL, _, err := s.forgeCreatePR(ctx, org, st.Repo, st.Branch, base, title, body) if err != nil { if st2, err := readStatus(wsDir); err == nil { st2.Question = fmt.Sprintf("PR creation failed: %v", err) diff --git a/pkg/agentic/mirror.go b/pkg/agentic/mirror.go index 615b9d9..86fe058 100644 --- a/pkg/agentic/mirror.go +++ b/pkg/agentic/mirror.go @@ -150,7 +150,8 @@ func (s *PrepSubsystem) mirror(ctx context.Context, _ *mcp.CallToolRequest, inpu // createGitHubPR creates a PR from dev → main using the gh CLI. func (s *PrepSubsystem) createGitHubPR(ctx context.Context, repoDir, repo string, commits, files int) (string, error) { // Check if there's already an open PR from dev - checkCmd := exec.CommandContext(ctx, "gh", "pr", "list", "--head", "dev", "--state", "open", "--json", "url", "--limit", "1") + ghRepo := fmt.Sprintf("%s/%s", GitHubOrg(), repo) + checkCmd := exec.CommandContext(ctx, "gh", "pr", "list", "--repo", ghRepo, "--head", "dev", "--state", "open", "--json", "url", "--limit", "1") checkCmd.Dir = repoDir out, err := checkCmd.Output() if err == nil && strings.Contains(string(out), "url") { @@ -175,6 +176,7 @@ func (s *PrepSubsystem) createGitHubPR(ctx context.Context, repoDir, repo string title := fmt.Sprintf("[sync] %s: %d commits, %d files", repo, commits, files) prCmd := exec.CommandContext(ctx, "gh", "pr", "create", + "--repo", ghRepo, "--head", "dev", "--base", "main", "--title", title, diff --git a/pkg/agentic/paths.go b/pkg/agentic/paths.go index 7aa5856..737878d 100644 --- a/pkg/agentic/paths.go +++ b/pkg/agentic/paths.go @@ -4,6 +4,7 @@ package agentic import ( "os" + "os/exec" "path/filepath" "strings" ) @@ -43,6 +44,27 @@ func AgentName() string { return "charon" } +// gitDefaultBranch detects the default branch of a repo (main, master, etc.). +func gitDefaultBranch(repoDir string) string { + cmd := exec.Command("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short") + cmd.Dir = repoDir + if out, err := cmd.Output(); err == nil { + ref := strings.TrimSpace(string(out)) + if strings.HasPrefix(ref, "origin/") { + return strings.TrimPrefix(ref, "origin/") + } + return ref + } + for _, branch := range []string{"main", "master"} { + cmd := exec.Command("git", "rev-parse", "--verify", branch) + cmd.Dir = repoDir + if cmd.Run() == nil { + return branch + } + } + return "main" +} + // GitHubOrg returns the GitHub org for mirror operations. func GitHubOrg() string { if org := os.Getenv("GITHUB_ORG"); org != "" { diff --git a/pkg/agentic/prep.go b/pkg/agentic/prep.go index 72cf1fb..a290c41 100644 --- a/pkg/agentic/prep.go +++ b/pkg/agentic/prep.go @@ -160,6 +160,11 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques // Create workspace structure // kb/ and specs/ will be created inside src/ after clone + // Ensure workspace directory exists + if err := os.MkdirAll(wsDir, 0755); err != nil { + return nil, PrepOutput{}, coreerr.E("prep", "failed to create workspace dir", err) + } + out := PrepOutput{WorkspaceDir: wsDir} // Source repo path diff --git a/pkg/agentic/scan.go b/pkg/agentic/scan.go index 54ae35f..7987188 100644 --- a/pkg/agentic/scan.go +++ b/pkg/agentic/scan.go @@ -127,9 +127,15 @@ func (s *PrepSubsystem) listOrgRepos(ctx context.Context, org string) ([]string, } func (s *PrepSubsystem) listRepoIssues(ctx context.Context, org, repo, label string) ([]ScanIssue, error) { - url := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues?state=open&labels=%s&limit=10&type=issues", - s.forgeURL, org, repo, label) - req, _ := http.NewRequestWithContext(ctx, "GET", url, nil) + u := fmt.Sprintf("%s/api/v1/repos/%s/%s/issues?state=open&limit=10&type=issues", + s.forgeURL, org, repo) + if label != "" { + u += "&labels=" + strings.ReplaceAll(strings.ReplaceAll(label, " ", "%20"), "&", "%26") + } + req, err := http.NewRequestWithContext(ctx, "GET", u, nil) + if err != nil { + return nil, coreerr.E("scan.listRepoIssues", "failed to create request", err) + } req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) diff --git a/pkg/agentic/verify.go b/pkg/agentic/verify.go index d14505f..146ee35 100644 --- a/pkg/agentic/verify.go +++ b/pkg/agentic/verify.go @@ -110,17 +110,19 @@ func (s *PrepSubsystem) attemptVerifyAndMerge(srcDir, org, repo, branch string, return mergeSuccess } -// rebaseBranch rebases the current branch onto origin/main and force-pushes. +// rebaseBranch rebases the current branch onto the default branch and force-pushes. func (s *PrepSubsystem) rebaseBranch(srcDir, branch string) bool { - // Fetch latest main - fetch := exec.Command("git", "fetch", "origin", "main") + base := gitDefaultBranch(srcDir) + + // Fetch latest default branch + fetch := exec.Command("git", "fetch", "origin", base) fetch.Dir = srcDir if err := fetch.Run(); err != nil { return false } - // Rebase onto main - rebase := exec.Command("git", "rebase", "origin/main") + // Rebase onto default branch + rebase := exec.Command("git", "rebase", "origin/"+base) rebase.Dir = srcDir if err := rebase.Run(); err != nil { // Rebase failed — abort and give up diff --git a/pkg/monitor/harvest.go b/pkg/monitor/harvest.go index 528a72d..2ab5dd7 100644 --- a/pkg/monitor/harvest.go +++ b/pkg/monitor/harvest.go @@ -20,6 +20,7 @@ import ( "dappco.re/go/agent/pkg/agentic" coreio "forge.lthn.ai/core/go-io" + coreerr "forge.lthn.ai/core/go-log" ) // harvestResult tracks what happened during harvest. @@ -260,7 +261,7 @@ func pushBranch(srcDir, branch string) error { cmd.Dir = srcDir out, err := cmd.CombinedOutput() if err != nil { - return fmt.Errorf("%s: %s", err, strings.TrimSpace(string(out))) + return coreerr.E("harvest.pushBranch", strings.TrimSpace(string(out)), err) } return nil } diff --git a/pkg/monitor/sync.go b/pkg/monitor/sync.go index 6c607bf..a327c5a 100644 --- a/pkg/monitor/sync.go +++ b/pkg/monitor/sync.go @@ -60,7 +60,8 @@ func (m *Subsystem) syncRepos() string { req.Header.Set("Authorization", "Bearer "+brainKey) } - resp, err := http.DefaultClient.Do(req) + client := &http.Client{Timeout: 15 * time.Second} + resp, err := client.Do(req) if err != nil { return "" } @@ -130,11 +131,15 @@ func (m *Subsystem) syncRepos() string { } } - // Only advance timestamp after attempting pulls — missed repos - // will be retried on the next cycle - m.mu.Lock() - m.lastSyncTimestamp = checkin.Timestamp - m.mu.Unlock() + // Only advance timestamp if we handled all reported repos. + // If any were skipped (dirty, wrong branch, missing), keep the + // old timestamp so the server reports them again next cycle. + skipped := len(checkin.Changed) - len(pulled) + if skipped == 0 { + m.mu.Lock() + m.lastSyncTimestamp = checkin.Timestamp + m.mu.Unlock() + } if len(pulled) == 0 { return ""