diff --git a/docs/cross-project-memory.md b/docs/cross-project-memory.md index 16f78eb..a850fc2 100644 --- a/docs/cross-project-memory.md +++ b/docs/cross-project-memory.md @@ -82,8 +82,8 @@ The memory subsystem exposes six MCP tools. All tools are listed by Optional: `date`, `tags` (string array), `related_doc`, `source_ref`, `confidence` (`low | medium | high`). - `set_agent_state` — upsert a keyed state value with optional `ttl_hours`. - Key prefix must match `owner` (e.g. `sansan.*`); the `team.*` prefix - requires `owner=eleven`. + Key prefix must match `owner` (for example, `agent-a.*`). The `team.*` + prefix requires `owner` to match `SCHIST_TEAM_OWNER`. - `delete_agent_state` — remove a keyed state value. The `owner` field on write is enforced against the configured agent @@ -121,6 +121,18 @@ Each write supplies its own `owner`; the call succeeds iff `owner` is in the allowlist. Per-entry attribution is preserved (every row keeps the calling agent's id in the `owner` column). +### Team state: `SCHIST_TEAM_OWNER` + +The shared `team.*` agent-state namespace is disabled unless a coordinator +owner is configured: + +```bash +SCHIST_TEAM_OWNER=coordinator +``` + +Only that owner can write or delete `team.*` keys. The same owner must also +be valid under `SCHIST_AGENT_ID` or `SCHIST_ALLOWED_AGENTS`. + ### Resolution order 1. `SCHIST_ALLOWED_AGENTS` defined → owner must be in the allowlist. Wins diff --git a/mcp-server/src/cli/memory-cli.ts b/mcp-server/src/cli/memory-cli.ts index 44a31b7..f5c30f8 100644 --- a/mcp-server/src/cli/memory-cli.ts +++ b/mcp-server/src/cli/memory-cli.ts @@ -14,6 +14,7 @@ * SCHIST_MEMORY_DB — path to agent-state.db (default: ~/.openclaw/memory/agent-state.db) * SCHIST_AGENT_ID — agent identity for write validation (single-agent mode) * SCHIST_ALLOWED_AGENTS — comma-separated allowlist (multi-agent shared deployments) + * SCHIST_TEAM_OWNER — owner allowed to write team.* state keys */ import * as sqliteReader from "../sqlite-reader.js"; @@ -26,7 +27,7 @@ COMMANDS add-memory --agent --type [options] "" Add a memory entry. - --agent required agent id (e.g. sansan, eleven, ninjia) + --agent required agent id (e.g. agent-a, coordinator) --type required entry_type: decision|lesson|blocker|completion|observation --date optional ISO date (default: today) --tags optional comma-separated tags @@ -42,7 +43,7 @@ COMMANDS --limit optional max results (default: 20) state get - Get an agent_state value by key (e.g. sansan.current_pr) + Get an agent_state value by key (e.g. agent-a.current_pr) state set "" --agent [--ttl N] Set an agent_state key. value is stored as JSON. @@ -53,6 +54,7 @@ ENVIRONMENT SCHIST_MEMORY_DB path to SQLite database (default: ~/.openclaw/memory/agent-state.db) SCHIST_AGENT_ID agent identity — enforced on writes (single-agent mode) SCHIST_ALLOWED_AGENTS comma-separated allowlist — overrides SCHIST_AGENT_ID (multi-agent shared deployments) + SCHIST_TEAM_OWNER owner allowed to write team.* state keys `); process.exit(1); } diff --git a/mcp-server/src/sqlite-reader.ts b/mcp-server/src/sqlite-reader.ts index 03223db..7271376 100644 --- a/mcp-server/src/sqlite-reader.ts +++ b/mcp-server/src/sqlite-reader.ts @@ -736,14 +736,25 @@ function openMemoryDb(): Database.Database { return db; } -/** Validate agent_state key prefix (Ninjia fix) */ +function getTeamStateOwner(): string | null { + const value = process.env.SCHIST_TEAM_OWNER?.trim(); + return value || null; +} + +/** Validate agent_state key namespace. */ function assertKeyPrefix(key: string, owner: string): void { const keyPrefix = key.split(".")[0]; if (keyPrefix !== owner && keyPrefix !== "team") { throw Object.assign(new Error(`agent_state: key '${key}' prefix must match owner '${owner}'`), { error: "VALIDATION_ERROR" }); } - if (keyPrefix === "team" && owner !== "eleven") { - throw Object.assign(new Error("agent_state: team.* keys require owner=eleven"), { error: "VALIDATION_ERROR" }); + if (keyPrefix === "team") { + const teamOwner = getTeamStateOwner(); + if (!teamOwner) { + throw Object.assign(new Error("agent_state: team.* keys require SCHIST_TEAM_OWNER to be configured"), { error: "VALIDATION_ERROR" }); + } + if (owner !== teamOwner) { + throw Object.assign(new Error("agent_state: team.* keys require owner to match SCHIST_TEAM_OWNER"), { error: "VALIDATION_ERROR" }); + } } } diff --git a/mcp-server/src/tool-registry.ts b/mcp-server/src/tool-registry.ts index 3c52a7b..d37b3ee 100644 --- a/mcp-server/src/tool-registry.ts +++ b/mcp-server/src/tool-registry.ts @@ -103,7 +103,7 @@ export function makeMemoryWriteTools(_config: VaultConfig) { }, { name: "set_agent_state", - description: "Set a keyed agent state value. Key prefix must match owner (e.g. sansan.*). team.* requires owner=eleven.", + description: "Set a keyed agent state value. Key prefix must match owner; team.* requires owner to match SCHIST_TEAM_OWNER.", inputSchema: { type: "object" as const, properties: { diff --git a/mcp-server/tests/memory.test.ts b/mcp-server/tests/memory.test.ts index e274bf1..bba0ada 100644 --- a/mcp-server/tests/memory.test.ts +++ b/mcp-server/tests/memory.test.ts @@ -12,11 +12,13 @@ beforeEach(async () => { process.env.SCHIST_MEMORY_DB = path.join(tempDir, "test-memory.db"); // Clear agent ID — individual tests set it as needed delete process.env.SCHIST_AGENT_ID; + delete process.env.SCHIST_TEAM_OWNER; }); afterEach(async () => { delete process.env.SCHIST_MEMORY_DB; delete process.env.SCHIST_AGENT_ID; + delete process.env.SCHIST_TEAM_OWNER; await fs.rm(tempDir, { recursive: true, force: true }); }); @@ -233,7 +235,7 @@ describe("searchMemory", () => { }); // --------------------------------------------------------------------------- -// set_agent_state — key prefix enforcement (Ninjia fix) +// set_agent_state — key prefix enforcement // --------------------------------------------------------------------------- describe("setAgentState", () => { @@ -248,14 +250,22 @@ describe("setAgentState", () => { expect(() => setAgentState("ninjia.current_task", "review", "sansan")).toThrow(); }); - it("allows team.* for owner=eleven", () => { - process.env.SCHIST_AGENT_ID = "eleven"; - const result = setAgentState("team.active_blockers", ["PR #266"], "eleven"); + it("allows team.* for configured team owner", () => { + process.env.SCHIST_AGENT_ID = "orchestrator"; + process.env.SCHIST_TEAM_OWNER = "orchestrator"; + const result = setAgentState("team.active_blockers", ["PR #266"], "orchestrator"); expect(result.key).toBe("team.active_blockers"); }); - it("rejects team.* for non-eleven owner", () => { + it("rejects team.* when no team owner is configured", () => { + process.env.SCHIST_AGENT_ID = "orchestrator"; + expect(() => setAgentState("team.active_blockers", [], "orchestrator")) + .toThrow(/SCHIST_TEAM_OWNER/); + }); + + it("rejects team.* for non-team owner", () => { process.env.SCHIST_AGENT_ID = "sansan"; + process.env.SCHIST_TEAM_OWNER = "orchestrator"; expect(() => setAgentState("team.active_blockers", [], "sansan")).toThrow(); }); @@ -281,7 +291,7 @@ describe("setAgentState", () => { // sansan tries to overwrite with a key that has ninjia prefix. // Since prefix check fires first for sansan->ninjia prefix, we need a // scenario where prefix passes but owner differs. - // Use team.* key: eleven sets it, then another eleven-impersonating agent + // Use team.* key: orchestrator sets it, then another orchestrator-impersonating agent // can't hijack. Actually the simplest: ninjia sets ninjia.x, then // another caller claiming to be ninjia but actually being someone else // via raw DB manipulation. But with assertOwner, the env var must match. @@ -291,22 +301,23 @@ describe("setAgentState", () => { // someone calls setAgentState with owner matching the new SCHIST_AGENT_ID // and a key that already exists with a different owner. // - // Simplest: use team.* keys — eleven creates team.x, then we change - // owner in DB to simulate another agent, and eleven tries to overwrite. + // Simplest: use team.* keys — orchestrator creates team.x, then we change + // owner in DB to simulate another agent, and orchestrator tries to overwrite. // Actually even simpler: directly test the ownership check. - // eleven sets team.shared - setStateAs("eleven", "team.shared", "eleven_data"); + // orchestrator sets team.shared + process.env.SCHIST_TEAM_OWNER = "orchestrator"; + setStateAs("orchestrator", "team.shared", "orchestrator_data"); // Manually change the owner in DB to simulate a different agent owning it const db = new Database(process.env.SCHIST_MEMORY_DB!); db.prepare("UPDATE agent_state SET owner = 'ninjia' WHERE key = 'team.shared'").run(); db.close(); - // Now eleven tries to overwrite — should fail with OWNERSHIP_ERROR - process.env.SCHIST_AGENT_ID = "eleven"; + // Now orchestrator tries to overwrite — should fail with OWNERSHIP_ERROR + process.env.SCHIST_AGENT_ID = "orchestrator"; try { - setAgentState("team.shared", "hijack", "eleven"); + setAgentState("team.shared", "hijack", "orchestrator"); fail("Expected error to be thrown"); } catch (e: unknown) { expect(e).toBeInstanceOf(Error);