diff --git a/go-api/internal/httpserver/definitions_api_test.go b/go-api/internal/httpserver/definitions_api_test.go index 4acbf18..7454da8 100644 --- a/go-api/internal/httpserver/definitions_api_test.go +++ b/go-api/internal/httpserver/definitions_api_test.go @@ -1520,3 +1520,84 @@ Find the shifts nobody has taken. t.Errorf("version moved on a restore: %v -> %v", arc["version"], got) } } + +// The reverse of the cycle and unknown-key checks. Those prove an edge is +// valid when the PARENT is written; this proves the edge stays valid when the +// CHILD is archived. Without it a published parent keeps delegating into +// nothing — exactly what krow-workforce-agent did after activity-agent was +// archived under it on 2026-09-15. +func TestArchivingADelegatedSubagentIsRefused(t *testing.T) { + r := newRBAC(t) + + agent := func(id, name, status string, version int, subagents ...string) string { + var sub string + if len(subagents) > 0 { + sub = "subagents:\n" + for _, s := range subagents { + sub += " - " + s + "\n" + } + } + return fmt.Sprintf(`--- +id: %s +name: %s +description: part of a delegation graph +status: %s +version: %d +pages: + - candidates +%s--- + +## Instructions +Delegate. +`, id, name, status, version, sub) + } + + res := r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{ + "markdown": agent("dep-child", "Child", "published", 1), "visibility": "organization", + }) + if res.code != http.StatusCreated { + t.Fatalf("create child: status %d (%v)", res.code, res.body) + } + childID, _ := res.record(t)["id"].(string) + + res = r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{ + "markdown": agent("dep-parent", "Parent", "published", 1, "dep-child"), "visibility": "organization", + }) + if res.code != http.StatusCreated { + t.Fatalf("create parent: status %d (%v)", res.code, res.body) + } + parentID, _ := res.record(t)["id"].(string) + + // Both ways of archiving must be refused: the status-only patch the UI + // sends, and a markdown save whose frontmatter says archived. + for name, patch := range map[string]map[string]any{ + "status patch": {"status": "archived"}, + "markdown save": {"markdown": agent("dep-child", "Child", "archived", 2)}, + } { + res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+childID, patch) + if res.code != http.StatusConflict { + t.Fatalf("%s: archiving a delegated-to agent: status %d, want 409 (%v)", name, res.code, res.body) + } + if body := fmt.Sprint(res.body); !strings.Contains(body, "dep-parent") { + t.Errorf("%s: the refusal did not name the dependent: %v", name, res.body) + } + } + + // Refused, not half-applied. + res = r.as(r.admin, "GET", "/api/v1/agent-definitions/"+childID, nil) + if st, _ := res.record(t)["status"].(string); st != "published" { + t.Fatalf("child status after refused archives = %q, want published", st) + } + + // A DRAFT parent does not pin the child. Move the parent to draft and the + // archive goes through: an abandoned experiment must not hold a + // production agent in place. + res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+parentID, map[string]any{"status": "draft"}) + if res.code != http.StatusOK { + t.Fatalf("draft the parent: status %d (%v)", res.code, res.body) + } + res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+childID, map[string]any{"status": "archived"}) + if res.code != http.StatusOK { + t.Fatalf("archive with only a draft dependent: status %d, want 200 (%v)", res.code, res.body) + } +} diff --git a/go-api/internal/service/definitions.go b/go-api/internal/service/definitions.go index f61d3a3..b274231 100644 --- a/go-api/internal/service/definitions.go +++ b/go-api/internal/service/definitions.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/url" + "sort" "strconv" "strings" @@ -352,6 +353,11 @@ func (s *DefinitionsService) UpdateAgent(ctx context.Context, ident authctx.Iden if err := s.refuseSubagentCycle(ctx, ident, agent.ID, agent.Subagents); err != nil { return nil, err } + if agent.Status == "archived" { + if err := s.refuseArchivingDependency(ctx, ident, agent.ID); err != nil { + return nil, err + } + } if err := s.refusePublishedRewrite(ctx, ident, repo.KindAgent, agent.ID, markdown, agent.Status, agent.Version); err != nil { return nil, err @@ -361,6 +367,12 @@ func (s *DefinitionsService) UpdateAgent(ctx context.Context, ident authctx.Iden if !isStr || (status != "draft" && status != "published" && status != "archived") { return nil, domain.Validation("status must be one of: draft, published, archived", map[string]string{"status": "invalid"}) } + if status == "archived" { + did, _ := existing["definition_id"].(string) + if err := s.refuseArchivingDependency(ctx, ident, did); err != nil { + return nil, err + } + } input.Status = &status } @@ -671,6 +683,70 @@ func (s *DefinitionsService) refuseSubagentCycle(ctx context.Context, return nil } +// refuseArchivingDependency fails an archive while a published agent in the +// organization still delegates to the definition. +// +// §3 says an unknown subagent key fails at publish, not at run time, and +// refuseSubagentCycle is half of that. This is the other half. Publish +// validation proves the edge exists when the PARENT is written; nothing +// re-checked it when the CHILD was later archived, so a spec could pass +// validation on Monday and be delegating into nothing by Friday. That is what +// happened to krow-workforce-agent on 2026-09-15: activity-agent was archived +// under it, and every run since logged runtime.unknown_subagent and answered +// activity questions without its activity capability — quietly, because the +// parent still Completed. +// +// Only published parents count. A draft that names this agent is the author's +// problem at their next publish, where rejectUnknownTools-style validation +// will tell them; refusing an archive on the strength of a draft would let an +// abandoned experiment pin a production agent in place forever. +// +// Unlike refuseSubagentCycle this FAILS CLOSED when the graph cannot be read. +// The cycle check can afford to fail open because the runtime depth cap holds +// regardless; the only backstop here is a parent that keeps answering with a +// capability missing, which is the failure this exists to prevent. +func (s *DefinitionsService) refuseArchivingDependency(ctx context.Context, + ident authctx.Identity, definitionID string) error { + + if definitionID == "" { + return nil + } + rows, _, err := s.repo.ListAgents(ctx, ident, repo.DefinitionListParams{ + Visibility: "organization", Limit: 500, + }) + if err != nil { + return domain.Internal(fmt.Errorf("could not check whether any published agent delegates to %q: %w", definitionID, err)) + } + + var dependents []string + for _, rec := range rows { + id, _ := rec["definition_id"].(string) + markdown, _ := rec["markdown"].(string) + status, _ := rec["status"].(string) + if id == "" || id == definitionID || markdown == "" || status != "published" { + continue + } + parsed, err := definition.ParseAgent(markdown, definition.Options{}) + if err != nil || parsed == nil { + continue + } + for _, sub := range parsed.Subagents { + if sub == definitionID { + dependents = append(dependents, id) + break + } + } + } + if len(dependents) == 0 { + return nil + } + sort.Strings(dependents) + return domain.Conflict(fmt.Sprintf( + "%s cannot be archived while a published agent delegates to it: %s. "+ + "Publish a version of each without it in `subagents` first, or archive them too.", + definitionID, strings.Join(dependents, ", "))) +} + // refusePublishedRewrite fails a publish that would change a version already // published, BEFORE anything is written. //