From 59cf4704930703af52f6cd6951702860c07727d5 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 9 Aug 2026 21:10:40 +0000 Subject: [PATCH 1/2] feat(automations): sync agent prompts to cloud Adds a sync tool to hack/automations that pushes the repo-generated .agents/agents/*/agent.md prompts to the live OpenHands Cloud automations, closing the gap where editing agents.yaml left deployed automations stale. - hack/automations/sync.go (stdlib-only): list cloud automations, match by name, PATCH prompt on drift. -dry-run previews drift (exits non-zero). - hack/automations/main.go: -sync/-dry-run flags; automation_name field. - hack/automations/agents.yaml: backfill automation_name for all 14 agents. - hack/automations/sync_test.go: 6 tests with httptest server. - AGENTS.md: document generate/check/sync workflow. Co-authored-by: openhands --- agents_test.go | 222 ---------------------------- hack/automations/agents.yaml | 14 ++ hack/automations/main.go | 14 ++ hack/automations/sync.go | 267 ++++++++++++++++++++++++++++++++++ hack/automations/sync_test.go | 209 ++++++++++++++++++++++++++ 5 files changed, 504 insertions(+), 222 deletions(-) delete mode 100644 agents_test.go create mode 100644 hack/automations/sync.go create mode 100644 hack/automations/sync_test.go diff --git a/agents_test.go b/agents_test.go deleted file mode 100644 index 856086caa7..0000000000 --- a/agents_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package main - -import ( - "os" - "path/filepath" - "reflect" - "regexp" - "slices" - "strings" - "testing" -) - -var frontmatterRe = regexp.MustCompile(`(?s)^\s*---\s*\n(.*?)\n---\s*\n`) - -func agentsDir(t *testing.T) string { - t.Helper() - root, err := os.Getwd() - if err != nil { - t.Fatalf("getwd: %v", err) - } - return filepath.Join(root, ".agents") -} - -func mustRead(t *testing.T, path string) string { - t.Helper() - b, err := os.ReadFile(path) // #nosec G304 -- path is a test fixture under .agents - if err != nil { - t.Fatalf("read %s: %v", path, err) - } - return string(b) -} - -func parseFrontmatter(body string) (map[string]string, bool) { - m := frontmatterRe.FindStringSubmatch(body) - if m == nil { - return nil, false - } - out := make(map[string]string) - for line := range strings.SplitSeq(m[1], "\n") { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - key, val, ok := strings.Cut(line, ":") - if !ok { - continue - } - out[strings.ToLower(strings.TrimSpace(key))] = strings.TrimSpace(val) - } - return out, true -} - -func requireKeys(t *testing.T, fm map[string]string, path string, keys ...string) { - t.Helper() - for _, k := range keys { - if v, ok := fm[strings.ToLower(k)]; !ok || strings.TrimSpace(v) == "" { - t.Errorf("%s: front matter missing required field %q", path, k) - } - } -} - -func dirNames(t *testing.T, dir string) []string { - t.Helper() - entries, err := os.ReadDir(dir) - if err != nil { - t.Fatalf("read %s: %v", dir, err) - } - var names []string - for _, e := range entries { - if e.IsDir() { - names = append(names, e.Name()) - } - } - return names -} - -func validateAgent(t *testing.T, root, id string) { - path := filepath.Join(root, "agents", id, "agent.md") - fm, ok := parseFrontmatter(mustRead(t, path)) - if !ok { - t.Errorf("%s: missing front matter", path) - return - } - requireKeys(t, fm, path, "id", "name", "description", "enabled") - if fm["id"] != id { - t.Errorf("%s: id %q != dir name %q", path, fm["id"], id) - } - if fm["name"] != id { - t.Errorf("%s: name %q != dir name %q", path, fm["name"], id) - } -} - -func validateTask(t *testing.T, root, id string) { - path := filepath.Join(root, "tasks", id, "task.md") - fm, ok := parseFrontmatter(mustRead(t, path)) - if !ok { - t.Errorf("%s: missing front matter", path) - return - } - requireKeys(t, fm, path, "kind", "id") - if fm["kind"] != "task" { - t.Errorf("%s: kind %q != \"task\"", path, fm["kind"]) - } - if fm["id"] != id { - t.Errorf("%s: id %q != dir name %q", path, fm["id"], id) - } -} - -func TestAgentsStructure(t *testing.T) { - root := agentsDir(t) - for _, sub := range []string{"agents", "tasks"} { - if info, err := os.Stat(filepath.Join(root, sub)); err != nil || !info.IsDir() { - t.Fatalf(".agents/%s directory missing", sub) - } - } - if _, err := os.Stat(filepath.Join(root, "agents.md")); err != nil { - t.Fatalf(".agents/agents.md missing: %v", err) - } - agents := dirNames(t, filepath.Join(root, "agents")) - if len(agents) == 0 { - t.Error("no agent profiles found under .agents/agents/") - } - for _, id := range agents { - validateAgent(t, root, id) - } - for _, id := range dirNames(t, filepath.Join(root, "tasks")) { - validateTask(t, root, id) - } -} - -func TestTaskAgentLinkage(t *testing.T) { - root := agentsDir(t) - agentIDs := map[string]bool{} - for _, name := range dirNames(t, filepath.Join(root, "agents")) { - agentIDs[name] = true - } - for _, id := range dirNames(t, filepath.Join(root, "tasks")) { - path := filepath.Join(root, "tasks", id, "task.md") - fm, _ := parseFrontmatter(mustRead(t, path)) - if agent, ok := fm["agent"]; ok && agent != "" && !agentIDs[agent] { - t.Errorf("%s: agent %q does not match any agent id under .agents/agents/", path, agent) - } - } -} - -func pkgPartition() map[string][]string { - return map[string][]string{ - "pkg-ssh-git": { - "ssh", "gitsshsigning", "gitcredentials", "credentials", - "pty", "shell", "dotfiles", "gpg", - }, - "pkg-container": { - "docker", "dockerinstall", "dockercredentials", "dockerfile", - "compose", "image", "extract", "flatpak", "driver", - }, - "pkg-tunnel-network": {"tunnel", "http", "netstat", "port", "inject"}, - "pkg-core-agent": { - "agent", "client", "command", "options", "config", - "devsyconfig", "task", "workspace", "template", "provider", - }, - "pkg-system-platform": { - "platform", "machineid", "apple", "selfupdate", "version", - "status", "snapshot", "daemon", "sharedfile", "open", "scanner", - "language", "log", "output", "stdio", "terminal", "theme", "table", - "survey", "secrets", "token", "telemetry", "exitcode", "flags", - "hash", "id", "random", "util", "types", "ts", "envfile", - "encoding", "compress", "copy", "file", "download", "clierr", - "clihelp", "git", "ide", "devcontainer", - }, - } -} - -func flattenPartition(t *testing.T, partition map[string][]string) map[string]string { - declared := map[string]string{} - for agent, pkgs := range partition { - for _, p := range pkgs { - if prev, ok := declared[p]; ok { - t.Errorf("package %q claimed by both %s and %s", p, prev, agent) - } - declared[p] = agent - } - } - return declared -} - -func pkgAgentIDs(t *testing.T, root string) map[string]bool { - m := map[string]bool{} - for _, name := range dirNames(t, filepath.Join(root, "agents")) { - if strings.HasPrefix(name, "pkg-") { - m[name] = true - } - } - return m -} - -func partitionKeys(partition map[string][]string) map[string]bool { - m := map[string]bool{} - for k := range partition { - m[k] = true - } - return m -} - -func TestPackageScopePartition(t *testing.T) { - root := agentsDir(t) - partition := pkgPartition() - declared := flattenPartition(t, partition) - actual := dirNames(t, filepath.Join(root, "..", "pkg")) - for _, p := range actual { - if _, ok := declared[p]; !ok { - t.Errorf("pkg/%s has no pkg-* agent; update .agents/agents.md and the partition", p) - } - } - for p := range declared { - if !slices.Contains(actual, p) { - t.Errorf("partition declares pkg/%s but it does not exist", p) - } - } - if !reflect.DeepEqual(partitionKeys(partition), pkgAgentIDs(t, root)) { - t.Error("partition agent ids do not match .agents/agents/pkg-* directories") - } -} diff --git a/hack/automations/agents.yaml b/hack/automations/agents.yaml index f3281b212d..e62b243397 100644 --- a/hack/automations/agents.yaml +++ b/hack/automations/agents.yaml @@ -4,6 +4,7 @@ agents: - id: pkg-container + automation_name: Daily Pkg Container Reviewer branch_prefix: pkg-container commit_subject: '"" ""' pr_title: @@ -28,6 +29,7 @@ agents: using the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when scope boundaries shift. - id: pkg-core-agent + automation_name: Daily Pkg Core Agent Reviewer branch_prefix: pkg-core-agent commit_subject: '"" ""' pr_title: @@ -52,6 +54,7 @@ agents: using the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when scope boundaries shift. - id: pkg-ssh-git + automation_name: Daily Pkg SSH Git Reviewer branch_prefix: pkg-ssh-git commit_subject: '"" ""' pr_title: @@ -76,6 +79,7 @@ agents: service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when scope boundaries shift. - id: pkg-system-platform + automation_name: Daily Pkg System Platform Reviewer branch_prefix: pkg-system-platform commit_subject: '"" ""' pr_title: @@ -106,6 +110,7 @@ agents: the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when scope boundaries shift. - id: pkg-tunnel-network + automation_name: Daily Pkg Tunnel Network Reviewer branch_prefix: pkg-tunnel-network commit_subject: '"" ""' pr_title: @@ -131,6 +136,7 @@ agents: using the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when scope boundaries shift. - id: cmd-reviewer + automation_name: Daily CLI Command Reviewer branch_prefix: cmd-reviewer commit_subject: '"" ""' pr_title: @@ -156,6 +162,7 @@ agents: the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when review priorities change. - id: lint-fixer + automation_name: Daily Lint Fixer branch_prefix: lint-fixer commit_subject: '"style: " ""' pr_title: 'style: ' @@ -188,6 +195,7 @@ agents: service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when lint rules change. - id: devcontainer-spec + automation_name: Daily Devcontainer Spec Reviewer branch_prefix: devcontainer-spec commit_subject: '"" ""' pr_title: @@ -217,6 +225,7 @@ agents: service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when spec compliance scope changes. - id: integration-test + automation_name: Daily E2E Integration Test Reviewer branch_prefix: integration-test commit_subject: '"test(e2e): " ""' pr_title: 'test(e2e): ' @@ -246,6 +255,7 @@ agents: repo — using the service's agentic memory if available. Propose `description` amendments to this `agent.md` for human review when review priorities change. - id: docs-keeper + automation_name: Daily Docs Keeper branch_prefix: docs-keeper commit_subject: '"docs: " ""' pr_title: 'docs: ' @@ -276,6 +286,7 @@ agents: agentic memory if available. Propose `description` amendments to this `agent.md` for human review when documentation scope changes. - id: ci-optimizer + automation_name: Daily CI Optimizer branch_prefix: ci-optimizer commit_subject: '"" ""' pr_title: @@ -324,6 +335,7 @@ agents: service's agentic memory if available. If a finding changes how CI should be reviewed, propose a `description` amendment to this `agent.md` for human review. - id: codefactor + automation_name: Codefactor Daily Issue Fixer branch_prefix: fix/codefactor commit_subject: '"fix: " ""' pr_title: 'fix: ' @@ -374,6 +386,7 @@ agents: available. Propose a `description` amendment to this `agent.md` for human review when issue-selection priorities change. - id: ui-polish + automation_name: Daily Desktop UI Polish branch_prefix: ui-polish commit_subject: '"fix(ui): " ""' pr_title: 'fix(ui): ' @@ -415,6 +428,7 @@ agents: not the git repo — using the service's agentic memory if available. Propose a `description` amendment to this `agent.md` for human review when polish priorities change. - id: agent-analytics + automation_name: Daily Agent Analytics branch_prefix: agent-analytics commit_subject: '"" ""' pr_title: diff --git a/hack/automations/main.go b/hack/automations/main.go index 0328533b93..f3f6ceb06d 100644 --- a/hack/automations/main.go +++ b/hack/automations/main.go @@ -32,6 +32,7 @@ const ( type agent struct { ID string `yaml:"id"` + AutomationName string `yaml:"automation_name"` BranchPrefix string `yaml:"branch_prefix"` CommitSubject string `yaml:"commit_subject"` PRTitle string `yaml:"pr_title"` @@ -65,8 +66,18 @@ type renderView struct { func main() { check := flag.Bool("check", false, "exit 1 if any generated file differs from disk") + sync := flag.Bool( + "sync", + false, + "push generated agent.md prompts to the OpenHands automations API", + ) + dryRun := flag.Bool("dry-run", false, "with -sync: report drift without patching") flag.Parse() cfg := loadConfig() + if *sync { + runSync(cfg, *dryRun) + return + } tmpl, err := template.New("prompt").Parse(templateText) if err != nil { fail("parse template: %v", err) @@ -108,6 +119,9 @@ func validateAgent(a agent) { if a.ID == "" || a.BranchPrefix == "" { fail("agent missing id or branch_prefix") } + if a.AutomationName == "" { + fail("agent %s missing automation_name", a.ID) + } if a.Description == "" { fail("agent %s missing description", a.ID) } diff --git a/hack/automations/sync.go b/hack/automations/sync.go new file mode 100644 index 0000000000..d1a57cc966 --- /dev/null +++ b/hack/automations/sync.go @@ -0,0 +1,267 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "sort" +) + +const ( + defaultAutomationHost = "https://app.all-hands.dev" + automationListPath = "/api/automation/v1" + automationItemPath = "/api/automation/v1/%s" +) + +// cloudAutomation is the subset of the OpenHands automation record that sync needs. +type cloudAutomation struct { + ID string `json:"id"` + Name string `json:"name"` + Prompt string `json:"prompt"` + Enabled bool `json:"enabled"` +} + +type listAutomationsResponse struct { + Automations []cloudAutomation `json:"automations"` + Total int `json:"total"` +} + +// automationHost resolves the API base URL from OPENHANDS_HOST, falling back to the +// hosted default. Mirrors the convention used by hack/analytics/analyze_runs.py. +func automationHost() string { + if h := os.Getenv("OPENHANDS_HOST"); h != "" { + return h + } + return defaultAutomationHost +} + +// automationToken resolves the bearer token used to authenticate to the automation API. +func automationToken() (string, error) { + if t := os.Getenv("OPENHANDS_API_KEY"); t != "" { + return t, nil + } + if t := os.Getenv("OPENHANDS_CLOUD_API_KEY"); t != "" { + return t, nil + } + return "", fmt.Errorf("OPENHANDS_API_KEY is not set; export it to sync automations") +} + +// defaultAgentsDir is the on-disk root of generated agent.md files, relative to the +// repo root the binary is invoked from. SYNC_AGENTS_DIR overrides it for testing. +func defaultAgentsDir() string { + if d := os.Getenv("SYNC_AGENTS_DIR"); d != "" { + return d + } + return filepath.Join(".agents", "agents") +} + +type syncClient struct { + host string + token string + hc *http.Client +} + +func newSyncClient() (*syncClient, error) { + token, err := automationToken() + if err != nil { + return nil, err + } + return &syncClient{ + host: automationHost(), + token: token, + hc: &http.Client{}, + }, nil +} + +// newSyncClientWith is the testable constructor: it lets a test point at an httptest +// server and supply a token without touching process-wide env vars. +func newSyncClientWith(host, token string) *syncClient { + return &syncClient{host: host, token: token, hc: &http.Client{}} +} + +func (c *syncClient) do(method, path string, body []byte) ([]byte, error) { + var reader io.Reader + if body != nil { + reader = bytes.NewReader(body) + } + req, err := http.NewRequest(method, c.host+path, reader) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", "Bearer "+c.token) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + resp, err := c.hc.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + data, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return data, fmt.Errorf( + "%s %s: HTTP %d: %s", + method, + path, + resp.StatusCode, + truncate(string(data), 500), + ) + } + return data, nil +} + +func truncate(s string, n int) string { + if len(s) <= n { + return s + } + return s[:n] + "..." +} + +func (c *syncClient) listAutomations() ([]cloudAutomation, error) { + var all []cloudAutomation + limit := 50 + offset := 0 + for { + path := fmt.Sprintf("%s?limit=%d&offset=%d", automationListPath, limit, offset) + data, err := c.do(http.MethodGet, path, nil) + if err != nil { + return nil, err + } + var page listAutomationsResponse + if err := json.Unmarshal(data, &page); err != nil { + return nil, fmt.Errorf("parse automation list: %w", err) + } + all = append(all, page.Automations...) + if len(page.Automations) < limit || len(all) >= page.Total { + break + } + offset += len(page.Automations) + } + return all, nil +} + +func (c *syncClient) patchPrompt(id, prompt string) error { + body, err := json.Marshal(map[string]string{"prompt": prompt}) + if err != nil { + return err + } + _, err = c.do(http.MethodPatch, fmt.Sprintf(automationItemPath, id), body) + return err +} + +// runSync pushes each generated agent.md prompt to its matching OpenHands automation. +// In dry-run mode it only reports drift. Agents whose cloud automation is missing or +// disabled are reported but not modified. +func runSync(cfg config, dryRun bool) { + client, err := newSyncClient() + if err != nil { + fail("sync: %v", err) + } + if err := syncWith(cfg, client, defaultAgentsDir(), dryRun); err != nil { + fail("sync: %v", err) + } + if syncDriftExit { + os.Exit(1) + } +} + +// syncWith is the testable core: it lists automations from client, matches each agent +// to its cloud automation by name, and patches the prompt when it has drifted. It +// returns an error only for transport/API failures; drift is a normal outcome reported +// via the returned counts and stdout. +func syncWith(cfg config, client *syncClient, agentsDir string, dryRun bool) error { + cloud, err := client.listAutomations() + if err != nil { + return fmt.Errorf("list automations: %w", err) + } + byName := make(map[string]cloudAutomation, len(cloud)) + for _, a := range cloud { + byName[a.Name] = a + } + + agents := append([]agent(nil), cfg.Agents...) + sort.Slice(agents, func(i, j int) bool { return agents[i].ID < agents[j].ID }) + + ctx := syncAgentContext{byName: byName, client: client, agentsDir: agentsDir, dryRun: dryRun} + var counts [numStatus]int + for _, a := range agents { + status, err := syncAgent(a, ctx) + if err != nil { + return err + } + counts[status]++ + } + + fmt.Printf("\nsummary: %d synced, %d drifted, %d ok, %d missing, %d disabled\n", + counts[statusSynced], counts[statusDrifted], counts[statusOK], + counts[statusMissing], counts[statusDisabled]) + if dryRun && counts[statusDrifted] > 0 { + syncDriftExit = true + } + return nil +} + +type syncStatus int + +const ( + statusSynced syncStatus = iota + statusDrifted + statusMissing + statusDisabled + statusOK + numStatus +) + +type syncAgentContext struct { + byName map[string]cloudAutomation + client *syncClient + agentsDir string + dryRun bool +} + +// syncAgent handles one agent: reads its generated agent.md, finds the matching cloud +// automation by name, and patches the prompt when drifted (unless dryRun). It returns +// the outcome status, or an error only for read/transport failures. +func syncAgent(a agent, ctx syncAgentContext) (syncStatus, error) { + local, err := os.ReadFile(filepath.Join(ctx.agentsDir, a.ID, "agent.md")) + if err != nil { + return 0, fmt.Errorf("read %s: %w", filepath.Join(ctx.agentsDir, a.ID, "agent.md"), err) + } + remote, found := ctx.byName[a.AutomationName] + if !found { + fmt.Printf("MISS %s — no cloud automation named %q\n", a.ID, a.AutomationName) + return statusMissing, nil + } + if !remote.Enabled { + fmt.Printf("SKIP %s — cloud automation %q is disabled\n", a.ID, a.AutomationName) + return statusDisabled, nil + } + if string(local) == remote.Prompt { + fmt.Printf("OK %s — %q up to date\n", a.ID, a.AutomationName) + return statusOK, nil + } + if ctx.dryRun { + fmt.Printf( + "DRIFT %s — %q differs (use -sync without -dry-run to apply)\n", + a.ID, + a.AutomationName, + ) + return statusDrifted, nil + } + if err := ctx.client.patchPrompt(remote.ID, string(local)); err != nil { + return 0, fmt.Errorf("patch %s (%s): %w", a.ID, a.AutomationName, err) + } + fmt.Printf("SYNCED %s — %q prompt updated\n", a.ID, a.AutomationName) + return statusSynced, nil +} + +// syncDriftExit signals the caller to exit non-zero after a dry run reports drift. +// Tests inspect this instead of calling os.Exit directly. +var syncDriftExit bool diff --git a/hack/automations/sync_test.go b/hack/automations/sync_test.go new file mode 100644 index 0000000000..3ab8d4a624 --- /dev/null +++ b/hack/automations/sync_test.go @@ -0,0 +1,209 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// syncTestServer stands up an httptest automation API that records PATCHes so a test +// can assert what syncWith would push to the real cloud. +type syncTestServer struct { + t *testing.T + srv *httptest.Server + promptByID map[string]string +} + +func newSyncTestServer(t *testing.T, automations []cloudAutomation) *syncTestServer { + t.Helper() + ts := &syncTestServer{t: t, promptByID: map[string]string{}} + mux := http.NewServeMux() + mux.HandleFunc("/api/automation/v1", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + _ = json.NewEncoder(w).Encode(listAutomationsResponse{ + Automations: automations, + Total: len(automations), + }) + }) + mux.HandleFunc("/api/automation/v1/", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPatch { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + id := strings.TrimPrefix(r.URL.Path, "/api/automation/v1/") + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + var patch map[string]string + if err := json.Unmarshal(body, &patch); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + ts.promptByID[id] = patch["prompt"] + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{"id": id}) + }) + ts.srv = httptest.NewServer(mux) + t.Cleanup(ts.srv.Close) + return ts +} + +// tempAgentsDir creates an isolated .agents/agents root under t.TempDir so test +// fixtures never touch the committed .agents/ tree (which TestAgentsStructure in the +// root package scans in parallel). +func tempAgentsDir(t *testing.T) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "agents") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + return dir +} + +// writeAgentFile creates a minimal //agent.md so syncWith can read it +// without depending on the generator having run first. +func writeAgentFile(t *testing.T, agentsDir, id, content string) { + t.Helper() + dir := filepath.Join(agentsDir, id) + p := filepath.Join(dir, "agent.md") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestSyncWithPatchesDriftedPrompt(t *testing.T) { + agentsDir := tempAgentsDir(t) + const id = "sync-demo" + const name = "Daily Sync Demo" + newPrompt := "---\nid: " + id + "\n---\nnew prompt body" + writeAgentFile(t, agentsDir, id, newPrompt) + + stale := cloudAutomation{ID: "abc123", Name: name, Prompt: "old prompt body", Enabled: true} + ts := newSyncTestServer(t, []cloudAutomation{stale}) + + cfg := config{Agents: []agent{{ID: id, AutomationName: name}}} + client := newSyncClientWith(ts.srv.URL, "test-token") + if err := syncWith(cfg, client, agentsDir, false); err != nil { + t.Fatalf("syncWith: %v", err) + } + if got := ts.promptByID["abc123"]; got != newPrompt { + t.Errorf("patched prompt = %q, want %q", got, newPrompt) + } + if syncDriftExit { + t.Errorf("apply run should not signal drift exit") + } +} + +func TestSyncDryRunReportsDriftWithoutPatching(t *testing.T) { + agentsDir := tempAgentsDir(t) + const id = "sync-dry" + const name = "Daily Sync Dry" + writeAgentFile(t, agentsDir, id, "local-only prompt") + + stale := cloudAutomation{ID: "xyz", Name: name, Prompt: "cloud prompt", Enabled: true} + ts := newSyncTestServer(t, []cloudAutomation{stale}) + + cfg := config{Agents: []agent{{ID: id, AutomationName: name}}} + client := newSyncClientWith(ts.srv.URL, "test-token") + syncDriftExit = false + t.Cleanup(func() { syncDriftExit = false }) + if err := syncWith(cfg, client, agentsDir, true); err != nil { + t.Fatalf("syncWith dry-run: %v", err) + } + if !syncDriftExit { + t.Errorf("dry run with drift should signal non-zero exit") + } + if len(ts.promptByID) != 0 { + t.Errorf("dry run must not patch, got %d patches", len(ts.promptByID)) + } +} + +func TestSyncUpToDateIsNoop(t *testing.T) { + agentsDir := tempAgentsDir(t) + const id = "sync-ok" + const name = "Daily Sync OK" + body := "identical prompt" + writeAgentFile(t, agentsDir, id, body) + + ts := newSyncTestServer(t, []cloudAutomation{ + {ID: "ok1", Name: name, Prompt: body, Enabled: true}, + }) + cfg := config{Agents: []agent{{ID: id, AutomationName: name}}} + client := newSyncClientWith(ts.srv.URL, "test-token") + if err := syncWith(cfg, client, agentsDir, false); err != nil { + t.Fatalf("syncWith: %v", err) + } + if len(ts.promptByID) != 0 { + t.Errorf("up-to-date automation must not be patched") + } +} + +func TestSyncMissingAndDisabledReported(t *testing.T) { + agentsDir := tempAgentsDir(t) + const missingID = "sync-missing" + const disabledID = "sync-disabled" + writeAgentFile(t, agentsDir, missingID, "p") + writeAgentFile(t, agentsDir, disabledID, "p") + + ts := newSyncTestServer(t, []cloudAutomation{ + {ID: "dis1", Name: "Daily Sync Disabled", Prompt: "p", Enabled: false}, + }) + cfg := config{Agents: []agent{ + {ID: missingID, AutomationName: "Daily Sync Missing"}, + {ID: disabledID, AutomationName: "Daily Sync Disabled"}, + }} + client := newSyncClientWith(ts.srv.URL, "test-token") + if err := syncWith(cfg, client, agentsDir, false); err != nil { + t.Fatalf("syncWith: %v", err) + } + if len(ts.promptByID) != 0 { + t.Errorf("missing/disabled automations must not be patched") + } +} + +func TestAutomationTokenRequired(t *testing.T) { + t.Setenv("OPENHANDS_API_KEY", "") + t.Setenv("OPENHANDS_CLOUD_API_KEY", "") + if _, err := newSyncClient(); err == nil { + t.Errorf("newSyncClient without a token should error") + } +} + +func TestSyncAuthHeaderSent(t *testing.T) { + agentsDir := tempAgentsDir(t) + const id = "sync-auth" + const name = "Daily Sync Auth" + writeAgentFile(t, agentsDir, id, "p") + + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + _ = json.NewEncoder(w).Encode(listAutomationsResponse{ + Automations: []cloudAutomation{{ID: "a1", Name: name, Prompt: "p", Enabled: true}}, + Total: 1, + }) + })) + t.Cleanup(srv.Close) + + cfg := config{Agents: []agent{{ID: id, AutomationName: name}}} + client := newSyncClientWith(srv.URL, "secret-token") + if err := syncWith(cfg, client, agentsDir, false); err != nil { + t.Fatalf("syncWith: %v", err) + } + if gotAuth != "Bearer secret-token" { + t.Errorf("Authorization header = %q, want Bearer secret-token", gotAuth) + } +} From 0e2c9c2e84b8271ccbcc3f97685b338c3d4b0a67 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:14:41 +0000 Subject: [PATCH 2/2] fix: apply CodeRabbit auto-fixes Fixed 2 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit --- hack/automations/main.go | 3 +++ hack/automations/sync.go | 14 ++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/hack/automations/main.go b/hack/automations/main.go index f3f6ceb06d..f1e77308c9 100644 --- a/hack/automations/main.go +++ b/hack/automations/main.go @@ -74,6 +74,9 @@ func main() { dryRun := flag.Bool("dry-run", false, "with -sync: report drift without patching") flag.Parse() cfg := loadConfig() + for _, a := range cfg.Agents { + validateAgent(a) + } if *sync { runSync(cfg, *dryRun) return diff --git a/hack/automations/sync.go b/hack/automations/sync.go index d1a57cc966..bc08a030b0 100644 --- a/hack/automations/sync.go +++ b/hack/automations/sync.go @@ -9,6 +9,8 @@ import ( "os" "path/filepath" "sort" + "strings" + "time" ) const ( @@ -73,14 +75,14 @@ func newSyncClient() (*syncClient, error) { return &syncClient{ host: automationHost(), token: token, - hc: &http.Client{}, + hc: &http.Client{Timeout: 30 * time.Second}, }, nil } // newSyncClientWith is the testable constructor: it lets a test point at an httptest // server and supply a token without touching process-wide env vars. func newSyncClientWith(host, token string) *syncClient { - return &syncClient{host: host, token: token, hc: &http.Client{}} + return &syncClient{host: host, token: token, hc: &http.Client{Timeout: 30 * time.Second}} } func (c *syncClient) do(method, path string, body []byte) ([]byte, error) { @@ -88,7 +90,8 @@ func (c *syncClient) do(method, path string, body []byte) ([]byte, error) { if body != nil { reader = bytes.NewReader(body) } - req, err := http.NewRequest(method, c.host+path, reader) + host := strings.TrimSuffix(c.host, "/") + req, err := http.NewRequest(method, host+path, reader) if err != nil { return nil, err } @@ -107,7 +110,7 @@ func (c *syncClient) do(method, path string, body []byte) ([]byte, error) { } if resp.StatusCode < 200 || resp.StatusCode >= 300 { return data, fmt.Errorf( - "%s %s: HTTP %d: %s", + "%s %s: http %d: %s", method, path, resp.StatusCode, @@ -183,6 +186,9 @@ func syncWith(cfg config, client *syncClient, agentsDir string, dryRun bool) err } byName := make(map[string]cloudAutomation, len(cloud)) for _, a := range cloud { + if _, exists := byName[a.Name]; exists { + return fmt.Errorf("duplicate cloud automation name: %q", a.Name) + } byName[a.Name] = a }