From 442d6ab7e3c4750dc176ee6bb14c54d08d4f320f Mon Sep 17 00:00:00 2001 From: mintaka Date: Mon, 31 Aug 2026 14:16:45 -0400 Subject: [PATCH] feat(compass-agent): validate spawn role against the closed Manager taxonomy (RIG-3074) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tightens and validates the role-on-spawn contract RIG-2726 (#762) wired end to end. No new proto field and no new tool param — this narrows the existing `role` value-space and enforces the closed set server-side. ### TypeScript (tool edge) - Narrow `spawnParameters.role` from a non-blank `type("string")` to the closed literal union `'supervisor' | 'owner' | 'manager'`, so the model sees the exact taxonomy in the JSON schema and an off-taxonomy or empty label is rejected structurally at the tool edge (a literal union renders into the schema; the old `.narrow` did not). The non-blank rule is no longer description-carried for `role` (it is now structural); handle and persona keep theirs. - `cli.ts`: emit one `console.error` when a role is set but its `prompts//SYSTEM.md` did not materialize, so a role-without-shipped-prompt degradation is visible instead of a silent fallback to the default block-0. ### Go server (authority) - Add `spawnableRoles` — a fixed server constant of the three roles beside `rootSupervisorRole` (a frozen product decision, not derived from the operator config bundle). - `SpawnAsAccount` validates `req.GetRole()` against it before `CreateAgent`, so an unknown or empty role is refused in-band `CodeInvalidArgument` and writes no row — and the guard sits ahead of the create/resume switch, so it covers the resume branch too. All three roles are spawnable (a spawned supervisor is parented and permitted). Rewrote the set-at-creation invariant comment: role is caller-selected but server-validated; prompt text still arrives only via the operator bundle; the store stays the provision-time source of record. ### Tests - TS: schema rejects off-taxonomy roles and accepts each of the three. - Go pgtest: empty and off-taxonomy roles rejected with `CodeInvalidArgument` and no row; each of the three accepted and threaded to the Provision wire; the idempotent-resume and crash-resume tests retry with a valid different role and still prove stored-role immutability. Every pre-existing agent-initiated spawn test that omitted a role now passes `manager` so it keeps exercising its own invariant. Verified against real Postgres (throwaway container). ### Open questions - Second creation door: `CommsService.CreateAgent` (`go/internal/comms/comms.go:146-150`) calls `store.CreateAgent` with a `ParentAgentID` but no `Role`, so it can create roleless tree nodes — which the frozen taxonomy tenet 3 and DL-new-B say cannot exist. T1 only guards `SpawnAsAccount` (the `agents_spawn_peer` path DL-new-B scopes). Closing the second door is a compass-server design fork that pairs with RIG-2673 (agent-tool exposure of `create_agent`); filed as RIG-3097 for compass-server, not fixed here. Spec-impact: none. Refs RIG-3074 Co-authored-by: Matt Wilkinson --- go/server/dm_e2e_pgtest_test.go | 1 + go/server/lifecycle.go | 29 +++- go/server/lifecycle_e2e_pgtest_test.go | 2 + go/server/lifecycle_pgtest_test.go | 18 +++ go/server/serve_seed.go | 13 ++ go/server/spawn_role_persona_pgtest_test.go | 131 +++++++++++++++---- packages/compass-agent/src/cli.ts | 10 ++ packages/compass-agent/src/lifecycle.test.ts | 61 ++++++++- packages/compass-agent/src/lifecycle.ts | 25 ++-- 9 files changed, 242 insertions(+), 48 deletions(-) diff --git a/go/server/dm_e2e_pgtest_test.go b/go/server/dm_e2e_pgtest_test.go index eaad21e7f..a37e8d544 100644 --- a/go/server/dm_e2e_pgtest_test.go +++ b/go/server/dm_e2e_pgtest_test.go @@ -395,6 +395,7 @@ func TestPeerDMSpawnPathDelivers(t *testing.T) { Handle: "mgr-peer", DisplayName: "Manager Peer", ClientRequestId: "spawn-t6-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount(manager) = %v, want success", err) diff --git a/go/server/lifecycle.go b/go/server/lifecycle.go index a89e2a1e1..5c4d8c747 100644 --- a/go/server/lifecycle.go +++ b/go/server/lifecycle.go @@ -151,6 +151,13 @@ var errHandleTaken = errors.New("handle already taken") // never a normal outcome — CodeInternal, never a silent success. var errCallerNotAgent = errors.New("resolved caller is not an agent account") +// errUnknownRole is the in-band cause when a spawn names a role outside the +// closed taxonomy (spawnableRoles), including an empty role: every spawned node +// carries a valid role, and the server is the authority on the set. The label, +// not the prompt text, is validated — prompt text still arrives only via the +// operator config bundle. CodeInvalidArgument. +var errUnknownRole = errors.New("unknown spawn role") + // SpawnAsAccount creates a peer agent owned by the caller's OWNER and brings it // online, running the same provision->placement->start->session chain a human // spawn takes. The new agent's owner is the caller agent's owner (F2), resolved @@ -186,6 +193,22 @@ func (l *lifecycleService) SpawnAsAccount( ctx, cancel := context.WithTimeout(ctx, spawnChainTimeout) defer cancel() + // Role validation: every spawned node carries a role from the closed + // taxonomy, and the server is the authority on the set. This is the first + // check in the chain — a structurally-invalid role is the cheapest possible + // rejection (a pure in-memory set lookup), so reject it before any store I/O + // and before the create/resume switch below, so the guard also covers the + // idempotent-resume branch and a malformed role always returns the same + // CodeInvalidArgument regardless of unrelated handle state. The LABEL is + // validated here, never the prompt text: the container's block-0 prompt + // still arrives only via the operator config bundle (prompts//SYSTEM.md), + // so a valid label with an unshipped prompt degrades to the default block-0 + // (a visible runtime warn, not a spawn failure), while an off-taxonomy label + // never reaches the store at all. + if _, ok := spawnableRoles[req.GetRole()]; !ok { + return nil, connect.NewError(connect.CodeInvalidArgument, errUnknownRole) + } + // F2 ownership: the spawned peer inherits the CALLER'S OWNER. Resolve it // from the store — the caller is an agent account, and its owner is who the // new peer belongs to. @@ -216,10 +239,10 @@ func (l *lifecycleService) SpawnAsAccount( // Persona and role are set-at-creation from the spawn request (org-management // Manager creation): under the D9 owner-acts model the caller's OWNER is the // authority, so a Manager-creating spawn legitimately carries role+persona at - // creation. They are stored via CreateAgent (the source of record) and then + // creation. Role is caller-SELECTED but server-VALIDATED (above); persona is + // free-text. Both are stored via CreateAgent (the source of record) and then // threaded to the Runner from the CREATED store account below, never from the - // request directly — so an empty role+persona spawn is byte-identical to - // today's field-less spawn. + // request directly. created, err := l.store.CreateAgent(ctx, callerOwner, store.NewAgent{ Handle: req.GetHandle(), DisplayName: req.GetDisplayName(), diff --git a/go/server/lifecycle_e2e_pgtest_test.go b/go/server/lifecycle_e2e_pgtest_test.go index a4b346ad5..55baad005 100644 --- a/go/server/lifecycle_e2e_pgtest_test.go +++ b/go/server/lifecycle_e2e_pgtest_test.go @@ -148,6 +148,7 @@ func e2eSpawnHappyPath(t *testing.T, w *e2eWire) (peerID store.AccountID, peerCo Handle: "peer-1", DisplayName: "Peer One", ClientRequestId: "spawn-req-1", + Role: "manager", }}, })) if err != nil { @@ -368,6 +369,7 @@ func TestForeignOwnerDespawnOverTheWireIsIndistinguishableNoOp(t *testing.T) { Handle: "peer-b", DisplayName: "Peer B", ClientRequestId: "spawn-b-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount(agent B) = %v, want success", err) diff --git a/go/server/lifecycle_pgtest_test.go b/go/server/lifecycle_pgtest_test.go index c94c90d6e..55dc23015 100644 --- a/go/server/lifecycle_pgtest_test.go +++ b/go/server/lifecycle_pgtest_test.go @@ -85,6 +85,7 @@ func TestSpawnInheritsCallerOwner(t *testing.T) { Handle: "peer-1", DisplayName: "Peer One", ClientRequestId: "spawn-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount = %v, want success", err) @@ -135,6 +136,7 @@ func TestSpawnSetsParentToCaller(t *testing.T) { Handle: "peer-parent", DisplayName: "Peer Parent", ClientRequestId: "spawn-parent", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount = %v, want success", err) @@ -178,6 +180,7 @@ func TestSpawnSameClientRequestIdRetryJoins(t *testing.T) { first, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dup", ClientRequestId: "spawn-dup", + Role: "manager", }) if err != nil { t.Fatalf("first SpawnAsAccount = %v, want success", err) @@ -186,6 +189,7 @@ func TestSpawnSameClientRequestIdRetryJoins(t *testing.T) { second, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dup", ClientRequestId: "spawn-dup", + Role: "manager", }) if err != nil { t.Fatalf("retry SpawnAsAccount = %v, want idempotent success", err) @@ -232,6 +236,7 @@ func TestSpawnMidChainFailureRollsBack(t *testing.T) { _, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-roll", ClientRequestId: "spawn-roll", + Role: "manager", }) if err == nil { t.Fatal("SpawnAsAccount with a failing Start = nil error, want the failure surfaced") @@ -256,6 +261,7 @@ func TestSpawnMidChainFailureRollsBack(t *testing.T) { resp, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-roll", ClientRequestId: "spawn-roll-2", + Role: "manager", }) if err != nil { t.Fatalf("re-spawn of the same handle after rollback = %v, want success (handle must not be burned)", err) @@ -294,6 +300,7 @@ func TestSpawnSameHandleDifferentOwnerCreatesDistinctPeer(t *testing.T) { respA, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-shared", ClientRequestId: "spawn-a", + Role: "manager", }) if err != nil { t.Fatalf("owner-A spawn = %v, want success", err) @@ -313,6 +320,7 @@ func TestSpawnSameHandleDifferentOwnerCreatesDistinctPeer(t *testing.T) { respB, err := f.lc.SpawnAsAccount(ctx, callerB.ID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-shared", ClientRequestId: "spawn-b", + Role: "manager", }) if err != nil { t.Fatalf("owner-B spawn of the same handle = %v, want success (distinct per-owner peer)", err) @@ -362,6 +370,7 @@ func TestDespawnDifferentOwnerIsIndistinguishableNotFound(t *testing.T) { peerB, err := f.lc.SpawnAsAccount(ctx, callerB.ID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-b", ClientRequestId: "spawn-peer-b", + Role: "manager", }) if err != nil { t.Fatalf("owner-B spawn = %v, want success", err) @@ -421,6 +430,7 @@ func TestDespawnSameOwnerSiblingSucceeds(t *testing.T) { target, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "sibling", ClientRequestId: "spawn-sibling", + Role: "manager", }) if err != nil { t.Fatalf("spawn sibling = %v, want success", err) @@ -458,6 +468,7 @@ func TestDespawnSecondTimeIsIdempotentSuccess(t *testing.T) { target, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-twice", ClientRequestId: "spawn-twice", + Role: "manager", }) if err != nil { t.Fatalf("spawn = %v, want success", err) @@ -591,6 +602,7 @@ func TestSpawnHandleCollidesWithUserAccountIsAlreadyExists(t *testing.T) { _, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "taken-handle", ClientRequestId: "spawn-collides-user", + Role: "manager", }) if err == nil { t.Fatal("spawn onto a user-held handle = success, want CodeAlreadyExists (never resume/steal, never leak account kind)") @@ -624,6 +636,7 @@ func TestSpawnHandleCollidesWithSystemAccountIsAlreadyExists(t *testing.T) { _, err = f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: sys.Handle, ClientRequestId: "spawn-collides-system", + Role: "manager", }) if err == nil { t.Fatal("spawn onto the system handle = success, want CodeAlreadyExists (never shadow the system sender)") @@ -653,6 +666,7 @@ func TestSpawnAutoOpensManagerPeerDM(t *testing.T) { Handle: "peer-dm", DisplayName: "Peer DM", ClientRequestId: "spawn-dm-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount = %v, want success", err) @@ -696,6 +710,7 @@ func TestSpawnIdempotentReturnsSameDMName(t *testing.T) { first, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dm-idem", ClientRequestId: "spawn-dm-idem-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount(first) = %v, want success", err) @@ -709,6 +724,7 @@ func TestSpawnIdempotentReturnsSameDMName(t *testing.T) { second, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dm-idem", ClientRequestId: "spawn-dm-idem-2", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount(second) = %v, want success", err) @@ -745,6 +761,7 @@ func TestSpawnDMOpenFailureNeverRollsBackSpawn(t *testing.T) { resp, err := lc.SpawnAsAccount(context.Background(), pf.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dm-fail", ClientRequestId: "spawn-dm-fail-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount = %v, want success despite the DM-open failure (never a rollback)", err) @@ -765,6 +782,7 @@ func TestSpawnDMOpenFailureNeverRollsBackSpawn(t *testing.T) { resp, err := lc.SpawnAsAccount(context.Background(), pf.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dm-nil", ClientRequestId: "spawn-dm-nil-1", + Role: "manager", }) if err != nil { t.Fatalf("SpawnAsAccount = %v, want success with a nil DM opener", err) diff --git a/go/server/serve_seed.go b/go/server/serve_seed.go index 576221f36..99e7bdafa 100644 --- a/go/server/serve_seed.go +++ b/go/server/serve_seed.go @@ -27,6 +27,19 @@ const ( rootSupervisorRole = "manager" ) +// spawnableRoles is the closed Manager-role taxonomy a spawn request may name: +// supervisor (owns the whole tree — intake, incidents, broadcasts, first +// contact), owner (owns a product/service/domain), manager (owns one lane). The +// set is a frozen product decision, NOT derived from the operator config +// bundle's prompts/ members — legality is a fixed contract while the bundle is +// mutable state. All three are spawnable; a spawned supervisor is parented and +// permitted (standing up a separate tree is the intended use). +var spawnableRoles = map[string]struct{}{ + "supervisor": {}, + "owner": {}, + "manager": {}, +} + // seedClientRequestID is the fixed idempotency key the seed's SpawnAgent runs // under. Fixed (not per-call) so a re-enroll that re-fires the seed for an // already-seeded-and-live supervisor joins the completed spawn or is rejected diff --git a/go/server/spawn_role_persona_pgtest_test.go b/go/server/spawn_role_persona_pgtest_test.go index 29092a34e..85f5be3f3 100644 --- a/go/server/spawn_role_persona_pgtest_test.go +++ b/go/server/spawn_role_persona_pgtest_test.go @@ -13,15 +13,20 @@ package server import ( "context" + "errors" "testing" + "connectrpc.com/connect" + compassv1internal "github.com/RigelBuild/compass/go/internal/gen/compass/v1" "github.com/RigelBuild/compass/go/internal/store" ) -// TestSpawnStoresAndThreadsRole: a spawn with role "manager" lands the role in -// the created agent_accounts row AND on the Provision wire (threaded from the -// store account, the source of record). +// TestSpawnStoresAndThreadsRole: a spawn with role "manager" and no persona +// lands the role in the created agent_accounts row AND on the Provision wire +// (threaded from the store account, the source of record), and stores an empty +// persona verbatim — pinning the PR's deliberate asymmetry: role is a closed +// server-validated set, persona is free-text and may be empty. func TestSpawnStoresAndThreadsRole(t *testing.T) { f := newLifecycleFixture(t) ctx := context.Background() @@ -47,6 +52,9 @@ func TestSpawnStoresAndThreadsRole(t *testing.T) { if got := f.runner.provisionRole(t); got != "manager" { t.Fatalf("Provision wire role = %q, want manager (threaded from the store account)", got) } + if got := acc.Agent.Persona; got != "" { + t.Fatalf("stored persona = %q, want empty (a role-only spawn leaves persona blank; persona is free-text and optional)", got) + } } // TestSpawnStoresAndThreadsPersona: a spawn with a persona lands it in the @@ -60,6 +68,7 @@ func TestSpawnStoresAndThreadsPersona(t *testing.T) { Handle: "peer-persona", DisplayName: "Peer Persona", ClientRequestId: "spawn-persona", + Role: "manager", Persona: wantPersona, }) if err != nil { @@ -79,38 +88,98 @@ func TestSpawnStoresAndThreadsPersona(t *testing.T) { } } -// TestSpawnEmptyRolePersonaIsByteIdenticalToToday: a spawn naming neither role -// nor persona stores empty strings AND carries empty strings on the Provision -// wire — the field-less spawn is unchanged from pre-T4 behavior. -func TestSpawnEmptyRolePersonaIsByteIdenticalToToday(t *testing.T) { +// TestSpawnEmptyRoleIsRejected: a spawn naming no role (empty) is refused +// CodeInvalidArgument and writes NO account row — every spawned node carries a +// role from the closed taxonomy. This pins the ROLE requirement only; the +// contrasting invariant that an empty PERSONA is accepted (persona is +// free-text, not a closed set) is pinned by TestSpawnStoresAndThreadsRole, +// which spawns a valid role with no persona and asserts the stored persona is +// empty. +func TestSpawnEmptyRoleIsRejected(t *testing.T) { f := newLifecycleFixture(t) ctx := context.Background() - resp, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ - Handle: "peer-empty", - DisplayName: "Peer Empty", - ClientRequestId: "spawn-empty", + _, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ + Handle: "peer-empty-role", + DisplayName: "Peer Empty Role", + ClientRequestId: "spawn-empty-role", + Persona: "compass-agent lane", }) - if err != nil { - t.Fatalf("SpawnAsAccount = %v, want success", err) + if err == nil { + t.Fatal("SpawnAsAccount with an empty role = nil error, want CodeInvalidArgument") } - newID := store.AccountID(resp.GetAgentAccountId()) + if got := connect.CodeOf(err); got != connect.CodeInvalidArgument { + t.Fatalf("empty-role spawn code = %v, want CodeInvalidArgument", got) + } + if !errors.Is(err, errUnknownRole) { + t.Fatalf("empty-role spawn cause = %v, want errUnknownRole", err) + } + if _, err := f.store.AgentByHandle(ctx, f.ownerAdmin, "peer-empty-role"); err == nil { + t.Fatal("an account was created for an empty-role spawn, want none (rejected before CreateAgent)") + } else if !errors.Is(err, store.ErrNotFound) { + t.Fatalf("AgentByHandle(peer-empty-role) = %v, want ErrNotFound (no row written)", err) + } +} - acc, err := f.store.GetAccount(ctx, newID) - if err != nil { - t.Fatalf("GetAccount(spawned) = %v", err) +// TestSpawnOffTaxonomyRoleIsRejected: a spawn naming a role outside the closed +// taxonomy is refused CodeInvalidArgument and writes NO account row. +func TestSpawnOffTaxonomyRoleIsRejected(t *testing.T) { + f := newLifecycleFixture(t) + ctx := context.Background() + + _, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ + Handle: "peer-director", + DisplayName: "Peer Director", + ClientRequestId: "spawn-director", + Role: "director", + Persona: "compass-agent lane", + }) + if err == nil { + t.Fatal("SpawnAsAccount with an off-taxonomy role = nil error, want CodeInvalidArgument") } - if got := acc.Agent.Role; got != "" { - t.Fatalf("stored role = %q, want empty for a field-less spawn", got) + if got := connect.CodeOf(err); got != connect.CodeInvalidArgument { + t.Fatalf("off-taxonomy-role spawn code = %v, want CodeInvalidArgument", got) } - if got := acc.Agent.Persona; got != "" { - t.Fatalf("stored persona = %q, want empty for a field-less spawn", got) + if !errors.Is(err, errUnknownRole) { + t.Fatalf("off-taxonomy-role spawn cause = %v, want errUnknownRole", err) } - if got := f.runner.provisionRole(t); got != "" { - t.Fatalf("Provision wire role = %q, want empty for a field-less spawn", got) + if _, err := f.store.AgentByHandle(ctx, f.ownerAdmin, "peer-director"); err == nil { + t.Fatal("an account was created for an off-taxonomy-role spawn, want none (rejected before CreateAgent)") + } else if !errors.Is(err, store.ErrNotFound) { + t.Fatalf("AgentByHandle(peer-director) = %v, want ErrNotFound (no row written)", err) } - if got := f.runner.provisionPersona(t); got != "" { - t.Fatalf("Provision wire persona = %q, want empty for a field-less spawn", got) +} + +// TestSpawnAcceptsEachTaxonomyRole: all three roles are spawnable (supervisor +// stays spawnable — a spawned supervisor is parented and permitted), and each +// lands in the created row AND on the Provision wire threaded from the store. +func TestSpawnAcceptsEachTaxonomyRole(t *testing.T) { + for _, role := range []string{"supervisor", "owner", "manager"} { + t.Run(role, func(t *testing.T) { + f := newLifecycleFixture(t) + ctx := context.Background() + + resp, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ + Handle: "peer-" + role, + DisplayName: "Peer " + role, + ClientRequestId: "spawn-" + role, + Role: role, + Persona: "compass-agent lane", + }) + if err != nil { + t.Fatalf("SpawnAsAccount(role=%q) = %v, want success", role, err) + } + acc, err := f.store.GetAccount(ctx, store.AccountID(resp.GetAgentAccountId())) + if err != nil { + t.Fatalf("GetAccount(spawned) = %v", err) + } + if acc.Agent.Role != role { + t.Fatalf("stored role = %q, want %q", acc.Agent.Role, role) + } + if got := f.runner.provisionRole(t); got != role { + t.Fatalf("Provision wire role = %q, want %q (threaded from the store account)", got, role) + } + }) } } @@ -135,11 +204,13 @@ func TestSpawnIdempotentReSpawnKeepsStoredRolePersona(t *testing.T) { t.Fatalf("first SpawnAsAccount = %v, want success", err) } - // A retry naming a DIFFERENT role/persona must not rewrite the stored values. + // A retry naming a DIFFERENT (but still valid-taxonomy) role/persona must not + // rewrite the stored values. "owner" is a valid member — the point is that the + // resume ignores the retry's values, not that the role is rejected. second, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-dup-rp", ClientRequestId: "spawn-dup-rp", - Role: "impostor", + Role: "owner", Persona: "different", }) if err != nil { @@ -207,11 +278,13 @@ func TestSpawnResumeReprovisionThreadsStoredRolePersona(t *testing.T) { // Re-spawn the same handle under a DISTINCT client_request_id (so it reaches // CreateAgent, conflicts on the handle, and resumes the unplaced account) - // naming DIFFERENT role/persona — the injection attempt the resume must ignore. + // naming a DIFFERENT (still valid-taxonomy) role/persona — the injection + // attempt the resume must ignore. "owner" is a valid member; the resume must + // re-provision under the stored first-spawn values, never the retry's. second, err := f.lc.SpawnAsAccount(ctx, f.agentID, &compassv1internal.SpawnPeerRequest{ Handle: "peer-resume-rp", ClientRequestId: "spawn-resume-rp-2", - Role: "impostor", + Role: "owner", Persona: "different", }) if err != nil { diff --git a/packages/compass-agent/src/cli.ts b/packages/compass-agent/src/cli.ts index 86807f824..304ecada1 100644 --- a/packages/compass-agent/src/cli.ts +++ b/packages/compass-agent/src/cli.ts @@ -761,6 +761,16 @@ export async function main( role, ) : undefined; + if (role && rolePrompt === undefined) { + // A role was selected but its prompt did not materialize (absent, empty, or + // unreadable file). The boot still degrades gracefully to the default + // block-0 above, but a role-without-shipped-prompt is an operator config + // gap the server-side taxonomy cannot catch (it validates the label, never + // the bundle), so surface it loudly rather than silently. + console.error( + `[compass-agent] role ${role} is set but no prompt was found at prompts/${role}/SYSTEM.md — falling back to the default block-0`, + ); + } // Fleet OMP config passthrough (RIG-1678, design compass-agent-config-passthrough // §CP-1/CP-2/CP-4), applied AFTER loadMountedConfig and BEFORE diff --git a/packages/compass-agent/src/lifecycle.test.ts b/packages/compass-agent/src/lifecycle.test.ts index fa2eaa7bc..6b2aca518 100644 --- a/packages/compass-agent/src/lifecycle.test.ts +++ b/packages/compass-agent/src/lifecycle.test.ts @@ -12,6 +12,7 @@ import { describe, expect, test } from "bun:test"; import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import { arkToWireSchema } from "@oh-my-pi/pi-ai/utils/schema"; import { ArkErrors, type Type } from "arktype"; import { create, @@ -468,6 +469,27 @@ describe("lifecycle parameter schemas", () => { expect(rejects(spawnParameters, { ...valid, role: " " })).toBe(true); }); + test("spawn rejects an off-taxonomy role", () => { + const valid = { + handle: "worker-a", + role: "manager", + persona: "compass-agent lane", + }; + expect(rejects(spawnParameters, { ...valid, role: "director" })).toBe(true); + expect(rejects(spawnParameters, { ...valid, role: "worker" })).toBe(true); + expect(rejects(spawnParameters, { ...valid, role: "Manager" })).toBe(true); + }); + + test("spawn accepts each of the three taxonomy roles", () => { + const valid = { + handle: "worker-a", + persona: "compass-agent lane", + }; + for (const role of ["supervisor", "owner", "manager"]) { + expect(rejects(spawnParameters, { ...valid, role })).toBe(false); + } + }); + test("spawn rejects a missing, empty, or whitespace-only persona", () => { const valid = { handle: "worker-a", @@ -481,16 +503,45 @@ describe("lifecycle parameter schemas", () => { expect(rejects(spawnParameters, { ...valid, persona: " " })).toBe(true); }); - // The `.narrow` non-blank rules do not survive into the JSON Schema the model - // is shown, so the descriptions are the only place a caller reads them — - // asserted here so dropping the rule from a description reddens rather than - // silently re-blinding the model while the runtime narrow still rejects. + // The `.narrow` non-blank rules on `handle`/`persona` do not survive into the + // JSON Schema the model is shown, so the descriptions are the only place a + // caller reads them — asserted here so dropping the rule from a description + // reddens rather than silently re-blinding the model while the runtime narrow + // still rejects. `role` is a literal union (its closed set IS in the schema), + // so it needs no such carry and is not asserted here. test("spawn descriptions carry the non-blank rule unrepresentable in JSON Schema", () => { expect(spawnParameters.get("handle").description).toContain("blank"); - expect(spawnParameters.get("role").description).toContain("blank"); expect(spawnParameters.get("persona").description).toContain("blank"); }); + // The counterpart to the carry test above: `role` needs no description carry + // precisely because its closed literal union renders into the model-facing + // JSON Schema. arktype emits the union as an `anyOf` of `const`s, which the + // SDK's `arkToWireSchema` — the same conversion `toolWireSchema` runs to build + // the schema the agent loop hands the model — collapses to a flat + // `{ type: "string", enum: [...] }` before the model sees it. So the model is + // shown the exact three taxonomy values as an enum. Asserted against that + // model-facing conversion (not arktype's raw pre-collapse output) so a + // regression that stopped rendering the set — an arktype bump, a change to the + // SDK collapse, or reverting `role` to a bare `type("string").narrow(...)` + // (dropping the literal union arktype renders) — reddens instead of silently + // re-blinding the model while the runtime union validation and the server + // closed-set guard still reject off-taxonomy roles. + test("spawn role renders its closed taxonomy into the JSON Schema the model sees", () => { + const wire = arkToWireSchema(spawnParameters) as { + properties: { role: { type?: string; enum?: string[] } }; + }; + const role = wire.properties.role; + // A flat `type: "string"` proves the collapse ran — arktype's raw union + // node carries no `type`, only `anyOf`. + expect(role.type).toBe("string"); + expect([...(role.enum ?? [])].sort()).toEqual([ + "manager", + "owner", + "supervisor", + ]); + }); + test("despawn rejects an empty or whitespace-only agent_handle", () => { expect(rejects(despawnParameters, {})).toBe(true); expect(rejects(despawnParameters, { agent_handle: "" })).toBe(true); diff --git a/packages/compass-agent/src/lifecycle.ts b/packages/compass-agent/src/lifecycle.ts index d9cadae38..76877415f 100644 --- a/packages/compass-agent/src/lifecycle.ts +++ b/packages/compass-agent/src/lifecycle.ts @@ -75,20 +75,23 @@ export class LifecycleBroker { /** Exported so a test can validate the wire contract the agent loop enforces. */ export const spawnParameters = type({ - // The non-blank bound is enforced at runtime but is NOT expressible in JSON - // Schema — arktype drops the `.narrow` predicate from the wire schema the - // model is shown, so the description carries the rule instead (see the - // comms.ts `postParameters` note). + // The non-blank bound on `handle`/`persona` is enforced at runtime but is NOT + // expressible in JSON Schema — arktype drops the `.narrow` predicate from the + // wire schema the model is shown, so their descriptions carry the rule instead + // (see the comms.ts `postParameters` note). `role` needs no such carry: it is a + // closed literal union, which DOES render into the JSON Schema, so the model + // sees the exact taxonomy and an off-taxonomy (or empty) label is rejected + // structurally at the tool edge — the server re-validates it as the authority. handle: type("string") .narrow((s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank")) .describe("The new peer's account handle (unique); must not be blank"), - role: type("string") - .narrow((s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank")) - .describe( - "The block-0 prompt selector for the spawned peer — selects " + - "config/prompts//SYSTEM.md. Required; must not be blank. Set at " + - "creation only (a spawn onto an existing handle keeps the stored role).", - ), + role: type("'supervisor' | 'owner' | 'manager'").describe( + "The Manager role for the spawned peer, from the closed taxonomy: " + + "supervisor (owns the whole tree), owner (owns a product/service/domain), " + + "or manager (owns one lane). Selects config/prompts//SYSTEM.md as the " + + "peer's block-0 prompt. Set at creation only (a spawn onto an existing " + + "handle keeps the stored role).", + ), persona: type("string") .narrow((s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank")) .describe(