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") + } +}