Refuse archiving an agent that a published agent still delegates to
§3 says an unknown subagent key fails at publish, not at run time, and refuseSubagentCycle enforced that in one direction only: the edge was checked when the PARENT was written, and nothing re-checked it when the CHILD was later archived. So a spec could validate on Monday and be delegating into nothing by Friday. That is what happened on 2026-09-15. activity-agent was archived while krow-workforce-agent v2 still listed it, and every run since logged runtime.unknown_subagent and answered activity questions without its activity capability -- quietly, because the parent still Completed. Both archive paths now refuse with 409 naming the dependents: the status-only patch the UI sends, and a markdown save whose frontmatter says archived. Only PUBLISHED parents count, so an abandoned draft cannot pin a production agent in place. Unlike the cycle check this fails closed when the graph cannot be read, because the only backstop here is the failure it exists to prevent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJvibeSc1JYXjatankqM1g
This commit is contained in:
@@ -1520,3 +1520,84 @@ Find the shifts nobody has taken.
|
|||||||
t.Errorf("version moved on a restore: %v -> %v", arc["version"], got)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"sort"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"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 {
|
if err := s.refuseSubagentCycle(ctx, ident, agent.ID, agent.Subagents); err != nil {
|
||||||
return nil, err
|
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,
|
if err := s.refusePublishedRewrite(ctx, ident, repo.KindAgent,
|
||||||
agent.ID, markdown, agent.Status, agent.Version); err != nil {
|
agent.ID, markdown, agent.Status, agent.Version); err != nil {
|
||||||
return nil, err
|
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") {
|
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"})
|
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
|
input.Status = &status
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -671,6 +683,70 @@ func (s *DefinitionsService) refuseSubagentCycle(ctx context.Context,
|
|||||||
return nil
|
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
|
// refusePublishedRewrite fails a publish that would change a version already
|
||||||
// published, BEFORE anything is written.
|
// published, BEFORE anything is written.
|
||||||
//
|
//
|
||||||
|
|||||||
Reference in New Issue
Block a user