update the archive options

go-api/internal/httpserver/definitions_api_test.go

@@ -8,6 +8,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/krow/krow-backend/go-api/internal/httpserver"
 )
 
 // Phase 4E — Backend CRUD APIs for authored Agent and Skill definitions.
@@ -1327,3 +1329,194 @@ Ask myself.
 		t.Fatal("an agent naming itself as its own subagent was published")
 	}
 }
+
+/* ── 8. Curated (built-in) agent protection ───────────────────────────────── */
+
+// agentMD builds a minimal valid agent definition for a given id.
+func agentMD(id, name string) string {
+	return fmt.Sprintf("---\nid: %s\nname: %s\nstatus: draft\nversion: 1\npages:\n  - candidates\n---\n\n## Instructions\nDo the thing.\n", id, name)
+}
+
+// TestCuratedAgentIsNotDeletable covers the protection the Agents list implies
+// but React alone cannot enforce.
+//
+// A curated agent is published by `importagents` as an ordinary organization
+// row, so nothing in the table distinguishes it from a shared agent somebody
+// authored — the distinguishing fact is that the deployment ships its spec.
+// Without a check at the endpoint, any operator with a terminal could delete
+// the definition the RUNTIME resolves from, leaving the agent in the list and
+// every run of it answering 404.
+func TestCuratedAgentIsNotDeletable(t *testing.T) {
+	a := newAPI(t, httpserver.WithCuratedAgents("curated-agent"))
+
+	// What importagents publishes: the curated spec, at organization visibility.
+	curated := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown":   agentMD("curated-agent", "Curated Agent"),
+		"visibility": "organization",
+	})
+	if curated.code != http.StatusCreated && curated.code != http.StatusOK {
+		t.Fatalf("publish curated agent: got %d", curated.code)
+	}
+	curatedID := curated.record(t)["id"].(string)
+
+	// The admin who may delete any other organization definition is refused
+	// this one.
+	del := a.do("DELETE", "/api/v1/agent-definitions/"+curatedID, nil)
+	if del.code != http.StatusForbidden {
+		t.Errorf("delete curated agent: got %d, want 403", del.code)
+	}
+
+	// And it is still there — refused, not deleted-then-reported.
+	after := a.do("GET", "/api/v1/agent-definitions/"+curatedID, nil)
+	if after.code != http.StatusOK {
+		t.Fatalf("curated agent after refused delete: got %d, want 200", after.code)
+	}
+	if got := after.record(t)["definition_id"]; got != "curated-agent" {
+		t.Errorf("curated agent definition_id = %v, want curated-agent", got)
+	}
+}
+
+// TestPersonalOverrideOfCuratedAgentStaysDeletable protects the revert path.
+//
+// "Revert to shipped" in the Agents list deletes the account's own definition
+// of a shipped id. Protecting by id alone would break it, so the guard is
+// scoped to the organization tier — this is the test that says so.
+func TestPersonalOverrideOfCuratedAgentStaysDeletable(t *testing.T) {
+	a := newAPI(t, httpserver.WithCuratedAgents("curated-agent"))
+
+	override := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown":   agentMD("curated-agent", "My Version"),
+		"visibility": "personal",
+	})
+	if override.code != http.StatusCreated && override.code != http.StatusOK {
+		t.Fatalf("create personal override: got %d", override.code)
+	}
+	overrideID := override.record(t)["id"].(string)
+
+	del := a.do("DELETE", "/api/v1/agent-definitions/"+overrideID, nil)
+	if del.code != http.StatusOK {
+		t.Errorf("delete personal override of a curated id: got %d, want 200", del.code)
+	}
+	after := a.do("GET", "/api/v1/agent-definitions/"+overrideID, nil)
+	if after.code != http.StatusNotFound {
+		t.Errorf("override after delete: got %d, want 404", after.code)
+	}
+}
+
+// TestCustomAgentDeleteIsIsolated is the isolation case: removing one custom
+// agent removes that agent and nothing else.
+func TestCustomAgentDeleteIsIsolated(t *testing.T) {
+	a := newAPI(t, httpserver.WithCuratedAgents("curated-agent"))
+
+	// A curated agent, a second custom agent, and a skill — none of which the
+	// delete below is about.
+	curated := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown": agentMD("curated-agent", "Curated Agent"), "visibility": "organization",
+	}).record(t)["id"].(string)
+	keep := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown": agentMD("keep-me", "Keep Me"), "visibility": "personal",
+	}).record(t)["id"].(string)
+	skill := a.do("POST", "/api/v1/skill-definitions", map[string]any{
+		"markdown": validSkillMD, "visibility": "personal",
+	}).record(t)["id"].(string)
+
+	target := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown": agentMD("remove-me", "Remove Me"), "visibility": "personal",
+	}).record(t)["id"].(string)
+
+	if got := a.do("DELETE", "/api/v1/agent-definitions/"+target, nil); got.code != http.StatusOK {
+		t.Fatalf("delete custom agent: got %d", got.code)
+	}
+
+	// Gone.
+	if got := a.do("GET", "/api/v1/agent-definitions/"+target, nil); got.code != http.StatusNotFound {
+		t.Errorf("removed agent: got %d, want 404", got.code)
+	}
+	// Everything else untouched.
+	for name, id := range map[string]string{"curated agent": curated, "other custom agent": keep} {
+		if got := a.do("GET", "/api/v1/agent-definitions/"+id, nil); got.code != http.StatusOK {
+			t.Errorf("%s after an unrelated delete: got %d, want 200", name, got.code)
+		}
+	}
+	if got := a.do("GET", "/api/v1/skill-definitions/"+skill, nil); got.code != http.StatusOK {
+		t.Errorf("skill after an unrelated agent delete: got %d, want 200", got.code)
+	}
+}
+
+// TestArchiveAndRestorePreserveTheSameAgent is the persistence half of Remove.
+//
+// Removing an authored agent archives it. That claim is only worth anything if
+// archiving keeps the row: the same uuid, the same definition_id and the same
+// Markdown, so restoring returns the agent somebody wrote rather than a new one
+// wearing its name. This asserts the round trip against the real endpoints.
+func TestArchiveAndRestorePreserveTheSameAgent(t *testing.T) {
+	a := newAPI(t, httpserver.WithCuratedAgents("curated-agent"))
+
+	const live = `---
+id: coverage-helper
+name: Coverage Helper
+status: published
+version: 3
+pages:
+  - candidates
+---
+
+## Instructions
+Find the shifts nobody has taken.
+`
+	created := a.do("POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown": live, "visibility": "personal",
+	})
+	if created.code != http.StatusCreated && created.code != http.StatusOK {
+		t.Fatalf("create agent: got %d", created.code)
+	}
+	rec := created.record(t)
+	id := rec["id"].(string)
+	definitionID := rec["definition_id"]
+
+	// Remove -> archive. Same row, same body, only the status moves.
+	archived := a.do("PATCH", "/api/v1/agent-definitions/"+id, map[string]any{
+		"markdown": strings.Replace(live, "status: published", "status: archived", 1),
+	})
+	if archived.code != http.StatusOK {
+		t.Fatalf("archive agent: got %d", archived.code)
+	}
+	arc := archived.record(t)
+	if arc["status"] != "archived" {
+		t.Errorf("status after remove = %v, want archived", arc["status"])
+	}
+	if arc["id"] != id || arc["definition_id"] != definitionID {
+		t.Errorf("identity changed on archive: %v/%v, want %s/%v",
+			arc["id"], arc["definition_id"], id, definitionID)
+	}
+	if !strings.Contains(arc["markdown"].(string), "Find the shifts nobody has taken.") {
+		t.Error("instructions were lost when the agent was archived")
+	}
+
+	// It is still there — removal is not deletion.
+	if got := a.do("GET", "/api/v1/agent-definitions/"+id, nil); got.code != http.StatusOK {
+		t.Fatalf("removed agent should still be readable: got %d, want 200", got.code)
+	}
+
+	// Restore -> the SAME agent, as a draft.
+	restored := a.do("PATCH", "/api/v1/agent-definitions/"+id, map[string]any{
+		"markdown": strings.Replace(live, "status: published", "status: draft", 1),
+	})
+	if restored.code != http.StatusOK {
+		t.Fatalf("restore agent: got %d", restored.code)
+	}
+	res := restored.record(t)
+	if res["status"] != "draft" {
+		t.Errorf("status after restore = %v, want draft", res["status"])
+	}
+	if res["id"] != id || res["definition_id"] != definitionID {
+		t.Errorf("restore created a different agent: %v/%v, want %s/%v",
+			res["id"], res["definition_id"], id, definitionID)
+	}
+	if !strings.Contains(res["markdown"].(string), "Find the shifts nobody has taken.") {
+		t.Error("instructions were lost on the round trip")
+	}
+	if got := res["version"]; got != arc["version"] {
+		t.Errorf("version moved on a restore: %v -> %v", arc["version"], got)
+	}
+}

go-api/internal/httpserver/server.go

@@ -35,6 +35,7 @@ import (
 	"github.com/krow/krow-backend/go-api/internal/auth"
 	"github.com/krow/krow-backend/go-api/internal/config"
 	"github.com/krow/krow-backend/go-api/internal/db"
+	"github.com/krow/krow-backend/go-api/internal/definition"
 	"github.com/krow/krow-backend/go-api/internal/knowledge"
 	"github.com/krow/krow-backend/go-api/internal/runtime"
 	"github.com/krow/krow-backend/go-api/internal/service"
@@ -109,6 +110,11 @@ type serverOptions struct {
 	// Not configuration: it describes the artefact, not the deployment, and an
 	// environment variable could disagree with the code it claims to describe.
 	version string
+
+	// curatedAgents replaces the set New would otherwise read from disk.
+	// nil means "read the configured directory"; an empty non-nil set means
+	// "protect nothing", which is a thing a test needs to be able to say.
+	curatedAgents map[string]bool
 }
 
 // WithBuildVersion records which build this is.
@@ -154,6 +160,21 @@ func WithClock(now func() time.Time) Option {
 // It does not weaken anything: the engine still loads agents through the same
 // loader, still runs them under the same budgets, and still authorizes through
 // the same principal. Only the model behind it changes.
+// WithCuratedAgents names the delete-protected agent ids directly.
+//
+// Production loads these from disk; this exists so a test can state its own
+// protected set without a directory, exactly as WithAgentEngine lets one
+// supply an engine without a model credential.
+func WithCuratedAgents(ids ...string) Option {
+	return func(o *serverOptions) {
+		set := make(map[string]bool, len(ids))
+		for _, id := range ids {
+			set[id] = true
+		}
+		o.curatedAgents = set
+	}
+}
+
 func WithAgentEngine(e *runtime.Engine) Option {
 	return func(o *serverOptions) { o.agents = e }
 }
@@ -232,6 +253,30 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option)
 	// rather than becoming an agent that silently cannot do what it claims.
 	s.definitions = s.definitions.WithToolCheck(toolRegistry.Known)
 
+	// The agents this deployment ships specs for, so DELETE refuses them at the
+	// endpoint rather than only in the list that renders the button.
+	//
+	// Read from the directory `importagents` publishes from, so the protected
+	// set is the published set by construction. A deployment without that
+	// directory protects nothing and says so here, once, at boot: silence would
+	// leave an operator believing in a guard that is not running.
+	curated := o.curatedAgents
+	if curated == nil {
+		loaded, err := definition.CuratedIDs(cfg.Agents.CuratedPath)
+		if err != nil {
+			return nil, fmt.Errorf("load curated agents: %w", err)
+		}
+		curated = loaded
+	}
+	if len(curated) == 0 {
+		log.Warn("no curated agent specs found; built-in agents are not delete-protected",
+			"path", cfg.Agents.CuratedPath)
+	} else {
+		log.Info("curated agents are delete-protected",
+			"count", len(curated), "ids", definition.SortedIDs(curated))
+	}
+	s.definitions = s.definitions.WithCuratedAgents(curated)
+
 	mux := http.NewServeMux()
 	mux.HandleFunc("GET /health", s.handleHealth)
 	s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) +