Enforce §3's monotonic version and its DAG requirement at publish

Two rules §3 states and nothing checked.

MONOTONIC. The rewrite guard added earlier compares content at ONE version
number, so republishing an OLDER number with the text originally published
under it looked like a no-op: nothing conflicted, nothing was refused, and the
live row silently reverted. The agent then reads v1 in the UI while the newest
thing anybody approved was v2. The test publishes v1, publishes v2, republishes
v1 byte-for-byte, and asserts both the refusal and that the live row is still
v2. Without the guard it answers 200 and the row goes back to version 1.

DAG. §3 says cycle detection runs at publish; only the runtime depth cap
existed. That cap means a cycle was never a safety problem — it was a budget
one. Every run entering the loop spends its whole allowance delegating in a
circle before terminating, and the author learns about it from a bill rather
than from the publish that created it.

definition.FindSubagentCycle is a pure function over id -> subagent ids, so it
is tested directly: chains, diamonds, self-reference, loops not involving the
first agent walked, and a 5000-long chain that would matter if this were
written to recurse carelessly. It REPORTS the cycle ("a -> b -> c -> a")
rather than merely detecting one, because an operator otherwise has to find it
by hand across a set of specs. The report is deterministic — a test runs it
fifty times over a graph with two cycles and requires the same answer, since Go
randomises map iteration and an error message that changes between identical
runs is one nobody trusts.

Wired into both publish paths. importagents has every spec in hand, which is
the only place that is cheaply true. The API builds the graph from
organization-visible agents plus the incoming definition standing in for its
stored self — otherwise an edit that CREATES a cycle is checked against the
version that did not have one and passes. Personal agents are excluded: they
are invisible to everyone else so cannot complete anyone else's loop, and
reading them would mean reading other people's drafts to validate your own.

An edge to an agent that is not in the set is ignored rather than reported.
That is a different failure with a different message
(runtime.unknown_subagent), and conflating them prints "cycle detected" for
what is actually a typo.

The cycle test bumps the version on the loop-closing edit. Without that the
rewrite guard refuses it for changing published text, the test passes for the
wrong reason, and it would keep passing with cycle detection deleted — which
is how it was first written, and what running it without the guard showed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJvibeSc1JYXjatankqM1g
This commit is contained in:
2026-08-29 14:43:49 +05:30
parent 4c29185c3b
commit 377948708b
5 changed files with 471 additions and 0 deletions

View File

@@ -1175,3 +1175,155 @@ The instructions, unchanged throughout.
res.code, res.body)
}
}
// TestPublishedVersionCannotGoBackwards covers §3's "monotonic".
//
// The rewrite guard only compares content at ONE version number, so an older
// number republished with the text that was originally published under it
// looked like a no-op: no conflict, nothing to refuse, and the live row
// silently reverted. The agent in the UI then reads v1 while the newest thing
// anybody approved was v2.
func TestPublishedVersionCannotGoBackwards(t *testing.T) {
r := newRBAC(t)
const v1 = `---
id: monotonic-agent
name: Monotonic Agent
description: published twice, then rolled back
status: published
version: 1
pages:
- candidates
---
## Instructions
The first version.
`
res := r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
"markdown": v1, "visibility": "personal",
})
if res.code != http.StatusCreated {
t.Fatalf("create v1: status %d (%v)", res.code, res.body)
}
id, _ := res.record(t)["id"].(string)
v2 := strings.Replace(strings.Replace(v1, "version: 1", "version: 2", 1),
"The first version.", "The second version.", 1)
res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+id, map[string]any{"markdown": v2})
if res.code != http.StatusOK {
t.Fatalf("publish v2: status %d (%v)", res.code, res.body)
}
// Back to v1, byte-for-byte what v1 said. Nothing here conflicts — which
// is exactly why it used to succeed.
res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+id, map[string]any{"markdown": v1})
if res.code != http.StatusConflict {
t.Fatalf("republishing v1 after v2: status %d, want 409 (%v)", res.code, res.body)
}
// And the live definition is still v2, not silently reverted.
res = r.as(r.admin, "GET", "/api/v1/agent-definitions/"+id, nil)
if got := fmt.Sprint(res.record(t)["version"]); got != "2" {
t.Errorf("live version = %s, want 2 — the refused publish rolled it back anyway", got)
}
}
// TestSubagentCycleIsRefusedAtPublish covers §3's DAG requirement.
//
// runtime.MaxDelegationDepth bounds a cycle that reaches run time, so this is
// not a safety hole — it is a budget one. Every run that entered the loop would
// spend its whole allowance delegating in a circle before terminating, and the
// person who wrote the loop would learn about it from a bill rather than from
// the publish that created it.
func TestSubagentCycleIsRefusedAtPublish(t *testing.T) {
r := newRBAC(t)
// version is a parameter so the loop-closing edit can BUMP it. Otherwise
// the rewrite guard refuses that edit for changing published text, the
// test passes for the wrong reason, and it would keep passing with cycle
// detection removed entirely.
agent := func(id, name 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: published
version: %d
pages:
- candidates
%s---
## Instructions
Delegate.
`, id, name, version, sub)
}
// A, with no subagents yet.
res := r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
"markdown": agent("cycle-a", "Cycle A", 1), "visibility": "organization",
})
if res.code != http.StatusCreated {
t.Fatalf("create A: status %d (%v)", res.code, res.body)
}
idA, _ := res.record(t)["id"].(string)
// B delegates to A. Still a DAG.
res = r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
"markdown": agent("cycle-b", "Cycle B", 1, "cycle-a"), "visibility": "organization",
})
if res.code != http.StatusCreated {
t.Fatalf("create B pointing at A: status %d, want 201 — a chain is not a cycle (%v)",
res.code, res.body)
}
// Now close the loop: A delegates to B.
res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+idA, map[string]any{
"markdown": agent("cycle-a", "Cycle A", 2, "cycle-b"),
})
if res.code != http.StatusUnprocessableEntity && res.code != http.StatusBadRequest {
t.Fatalf("closing the loop: status %d, want a validation failure (%v)", res.code, res.body)
}
if body := fmt.Sprint(res.body); !strings.Contains(body, "cycle") {
t.Errorf("the refusal did not mention a cycle: %v", res.body)
}
// A must be unchanged — refused, not half-applied.
res = r.as(r.admin, "GET", "/api/v1/agent-definitions/"+idA, nil)
if md, _ := res.record(t)["markdown"].(string); strings.Contains(md, "cycle-b") {
t.Error("the refused edit was applied anyway")
}
}
// A self-reference is the shortest cycle and the easiest to write by accident.
func TestSelfReferencingSubagentIsRefused(t *testing.T) {
r := newRBAC(t)
const md = `---
id: narcissus-agent
name: Narcissus Agent
description: names itself
status: published
version: 1
pages:
- candidates
subagents:
- narcissus-agent
---
## Instructions
Ask myself.
`
res := r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
"markdown": md, "visibility": "organization",
})
if res.code == http.StatusCreated {
t.Fatal("an agent naming itself as its own subagent was published")
}
}