From 4f8082417782a2e29a295fa2639b716326971fee Mon Sep 17 00:00:00 2001 From: yatul Date: Mon, 21 Sep 2026 14:28:58 +0400 Subject: [PATCH] feat(agents): let create opt out of background summoning with "summon": false (#1570) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a predefined agent with a description kicks off summoning in a background goroutine. It finishes 10-20 seconds after the create response and writes SOUL.md, IDENTITY.md, CAPABILITIES.md and the agent frontmatter. For a client that manages agents as code that is data loss. Such a client brings its own context files and writes them through agents.files.set as soon as create returns; summoning lands afterwards and replaces them. Nothing fails: no error from the API, nothing in the logs, and the agent quietly runs on generated text instead of the committed role. The only way to avoid it today is to poll the agent status until it leaves "summoning" — which means every automation has to know that summoning exists at all. POST /v1/agents now accepts an optional "summon" field. Absent or true keeps today's behaviour, so existing clients are unaffected. False creates the agent active with the seeded template files and starts no LLM call; summoning is still available afterwards via POST /v1/agents/{id}/resummon. The status fallback also now resets an explicitly requested "summoning" status when nothing will summon the agent: that status is only ever cleared by the summoner, so such an agent would otherwise stay stuck in it forever. Surface parity: CLI and WS RPC (agents.create) create agents without a description and never summoned, so they need no flag; the web UI create flow wants summoning and keeps the default. --- docs/07-bootstrap-skills-memory.md | 12 +++ docs/18-http-api.md | 2 +- internal/http/agents.go | 29 +++++- internal/http/agents_create_summon_test.go | 104 +++++++++++++++++++++ 4 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 internal/http/agents_create_summon_test.go diff --git a/docs/07-bootstrap-skills-memory.md b/docs/07-bootstrap-skills-memory.md index 104006e3..13926106 100644 --- a/docs/07-bootstrap-skills-memory.md +++ b/docs/07-bootstrap-skills-memory.md @@ -242,6 +242,18 @@ The LLM outputs structured XML with each file in a tagged block. Parsing is done **Why not `write_file`?** The `ContextFileInterceptor` blocks predefined file writes from chat by design. Bypassing it would create a security hole. Instead, the summoner writes directly to the store — one call, no tool iterations. +**Opting out: `"summon": false` on `POST /v1/agents`.** Summoning runs in the +background and finishes 10-20s after create returns, overwriting the agent's +context files and `frontmatter` when it does. That is what you want for an +agent described in prose, and exactly what you do not want for an agent kept +as code: such a client writes its own files through `agents.files.set` right +after create, and summoning silently replaces them — no error, nothing in the +logs, the agent simply runs on generated text instead of the committed one. +Passing `"summon": false` creates the agent `active` with the seeded template +files and starts no LLM call. The field is optional and defaults to true, so +clients that do not send it keep the current behaviour. Summoning can still be +triggered later via `POST /v1/agents/{id}/resummon`. + --- ## 8. Skills -- 5-Tier Hierarchy diff --git a/docs/18-http-api.md b/docs/18-http-api.md index cecd82c1..93ccf435 100644 --- a/docs/18-http-api.md +++ b/docs/18-http-api.md @@ -124,7 +124,7 @@ CRUD operations for agent management. Requires `X-GoClaw-User-Id` header for mul | Method | Path | Description | Auth | |--------|------|-------------|------| | `GET` | `/v1/agents` | List agents accessible by user | Bearer | -| `POST` | `/v1/agents` | Create new agent | Bearer | +| `POST` | `/v1/agents` | Create new agent (`"summon": false` skips background summoning) | Bearer | | `GET` | `/v1/agents/{id}` | Get agent by ID or key | Bearer | | `PUT` | `/v1/agents/{id}` | Update agent (owner only) | Bearer | | `DELETE` | `/v1/agents/{id}` | Delete agent (owner only) | Bearer | diff --git a/internal/http/agents.go b/internal/http/agents.go index 5debc69b..3bb14877 100644 --- a/internal/http/agents.go +++ b/internal/http/agents.go @@ -247,6 +247,17 @@ func (h *AgentsHandler) handleList(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{"agents": publicAgents}) } +// shouldSummonOnCreate reports whether creating an agent kicks off background +// LLM summoning, which generates its context files from the description. +// +// requested is the client's "summon" field (absent means true). Clients that +// manage agents as code pass false: they bring their own context files and +// write them immediately after create, while summoning lands ~10-20s later and +// overwrites them. +func shouldSummonOnCreate(requested bool, agentType, description string, haveSummoner bool) bool { + return requested && agentType == store.AgentTypePredefined && description != "" && haveSummoner +} + func (h *AgentsHandler) handleCreate(w http.ResponseWriter, r *http.Request) { userID := store.UserIDFromContext(r.Context()) locale := store.LocaleFromContext(r.Context()) @@ -258,6 +269,15 @@ func (h *AgentsHandler) handleCreate(w http.ResponseWriter, r *http.Request) { var createReq struct { store.AgentData GrantGatewayOperatorAccess bool `json:"grant_gateway_operator_access,omitempty"` + // Summon opts out of the background LLM summoning that normally runs + // for a predefined agent with a description. Default (absent) is true, + // so existing clients are unaffected. + // + // Clients that manage agents as code bring their own context files and + // write them right after create. Summoning finishes ~10-20s later and + // overwrites whatever is there, silently, so those clients either lose + // their files or have to poll the agent status before writing. + Summon *bool `json:"summon,omitempty"` } if !bindJSON(w, r, locale, &createReq) { return @@ -310,9 +330,14 @@ func (h *AgentsHandler) handleCreate(w http.ResponseWriter, r *http.Request) { // Check if predefined agent has a description for LLM summoning description := req.AgentDescription - if req.AgentType == store.AgentTypePredefined && description != "" && h.summoner != nil { + summonWanted := createReq.Summon == nil || *createReq.Summon + switch { + case shouldSummonOnCreate(summonWanted, req.AgentType, description, h.summoner != nil): req.Status = store.AgentStatusSummoning - } else if req.Status == "" { + case req.Status == "" || req.Status == store.AgentStatusSummoning: + // Never leave an agent in "summoning" when nothing will summon it: + // that status is only cleared by the summoner, so without one the + // agent would stay stuck there forever. req.Status = store.AgentStatusActive } diff --git a/internal/http/agents_create_summon_test.go b/internal/http/agents_create_summon_test.go new file mode 100644 index 00000000..c4918d00 --- /dev/null +++ b/internal/http/agents_create_summon_test.go @@ -0,0 +1,104 @@ +package http + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +func TestShouldSummonOnCreate(t *testing.T) { + cases := []struct { + name string + requested bool + agentType string + description string + haveSummoner bool + want bool + }{ + {"default for a described predefined agent", true, store.AgentTypePredefined, "a sarcastic Rust reviewer", true, true}, + {"opted out by the client", false, store.AgentTypePredefined, "a sarcastic Rust reviewer", true, false}, + {"no description to summon from", true, store.AgentTypePredefined, "", true, false}, + {"open agents are never summoned", true, store.AgentTypeOpen, "a sarcastic Rust reviewer", true, false}, + {"no summoner configured", true, store.AgentTypePredefined, "a sarcastic Rust reviewer", false, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := shouldSummonOnCreate(tc.requested, tc.agentType, tc.description, tc.haveSummoner); got != tc.want { + t.Fatalf("shouldSummonOnCreate = %v, want %v", got, tc.want) + } + }) + } +} + +// createAgentForSummonTest posts body to the create handler with a summoner +// configured, and returns the created agent's status. +// +// The summoner is deliberately built with nil dependencies: if the handler +// ever starts summoning when the client opted out, SummonAgent would reach +// for them, and the test fails loudly instead of silently passing. +func createAgentForSummonTest(t *testing.T, body string) (string, *gatewayOperatorAgentStore) { + t.Helper() + agents := &gatewayOperatorAgentStore{} + handler := &AgentsHandler{ + agents: agents, + defaultWorkspace: "/tmp/workspace", + summoner: NewAgentSummoner(agents, nil, nil, nil), + } + + req := httptest.NewRequest(http.MethodPost, "/v1/agents", bytes.NewReader([]byte(body))) + req = req.WithContext(gatewayOperatorContext()) + rr := httptest.NewRecorder() + + handler.handleCreate(rr, req) + if rr.Code != http.StatusCreated { + t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) + } + var response struct { + ID uuid.UUID `json:"id"` + Status string `json:"status"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { + t.Fatalf("decode response: %v body=%s", err, rr.Body.String()) + } + return response.Status, agents +} + +func TestAgentsCreateSummonFalseGoesStraightToActive(t *testing.T) { + status, agents := createAgentForSummonTest(t, `{ + "agent_key":"scout", + "display_name":"Scout", + "provider":"anthropic", + "model":"claude", + "agent_description":"reads the morning news and writes a digest", + "summon":false + }`) + + // Asserting the status is enough to prove summoning never started, with no + // need to wait for it: handleCreate starts the goroutine if and only if the + // status it decided synchronously is "summoning" (`if req.Status == + // store.AgentStatusSummoning { go h.summoner.SummonAgent(...) }`). So a + // persisted "active" status means the goroutine was not spawned. + if status != store.AgentStatusActive { + t.Fatalf("status=%q, want %q: a client that brings its own context files must not wait for summoning", + status, store.AgentStatusActive) + } + if len(agents.agents) != 1 { + t.Fatalf("expected exactly one created agent, got %d", len(agents.agents)) + } + if got := agents.agents[0].Status; got != store.AgentStatusActive { + t.Fatalf("stored status=%q, want %q", got, store.AgentStatusActive) + } +} + +func TestAgentsCreateWithoutSummonFieldKeepsSummoning(t *testing.T) { + // No "summon" field: existing clients must keep the old behaviour. + if !shouldSummonOnCreate(true, store.AgentTypePredefined, "reads the morning news", true) { + t.Fatal("omitting the summon field must still summon a described predefined agent") + } +}