diff --git a/packages/core/src/mcp/tools.ts b/packages/core/src/mcp/tools.ts index a08ad128..b1bb0276 100644 --- a/packages/core/src/mcp/tools.ts +++ b/packages/core/src/mcp/tools.ts @@ -752,6 +752,9 @@ const agentFields = { ), }; +/** agentFields that change how an agent looks and is introduced, not what it can do. */ +const COSMETIC_AGENT_FIELDS = new Set(["role", "description", "avatar", "color", "character", "personality"]); + const triggerSchema = z .discriminatedUnion("type", [ z.object({ @@ -1631,10 +1634,11 @@ const TOOLS: ToolDef[] = [ defineTool({ name: "human_task_cancel", - description: "Take back a task you gave the human when you don't need it anymore (you found another way, or the work was dropped). It leaves their list.", + description: + "Take back a task you gave the human when you don't need it anymore (you found another way, or the work was dropped). It leaves their list. A manager may also take back a duplicate or obsolete one an agent of its team gave (one that reports to it, directly or further down), with a reason; the human's own tasks stay theirs.", schema: z.object({ id: z.string().min(1).max(100).describe('The task\'s id or its ref, e.g. "H-12"'), - reason: z.string().max(500).optional().describe("Why it isn't needed anymore"), + reason: z.string().max(500).optional().describe("Why it isn't needed anymore; required for a task of your team, the human sees it on the task"), }), when: (agent, ctx) => canAsk(agent, ctx) && followupsAllowed(ctx.conversationId), run: ({ id, reason }, { agent }) => { @@ -2124,19 +2128,22 @@ const TOOLS: ToolDef[] = [ defineTool({ name: "agent_update", description: - "Update an agent's name, role, who it reports to, look (emoji, colour, character), personality, description, instructions, model, delegation settings, browser on/off, MCP servers (within its scope), subagents, heartbeat or project (within its workspace). Browser profile: only a new, empty one from browser_profile_create (or null to put it back on the default from one); existing profiles stay the human's. Workspace, secret access and login permissions can only be changed by the human in Settings.", + "Update an agent's name, role, who it reports to, look (emoji, colour, character), personality, description, instructions, model, delegation settings, browser on/off, MCP servers (within its scope), subagents, heartbeat or project (within its workspace). Browser profile: only a new, empty one from browser_profile_create (or null to put it back on the default from one); existing profiles stay the human's. Workspace, secret access and login permissions can only be changed by the human in Settings. On an agent of another workspace that can reveal secrets, the built-in agent may set only role, description, avatar, color, character and personality, with no other field in the same call.", schema: z.object({ agentId: z.string(), name: z.string().min(1).max(100).optional(), ...agentFields }), when: managesSetup, run: async ({ agentId, ...patch }, { agent, ctx }) => { const target = getAgent(agentId); + // How a reveal-mode agent of another workspace shows on the chart is the built-in agent's to tidy up, from a run that reads secrets itself. + const fields = Object.keys(patch); + const cosmetic = agent.isDefault && revealsHere(agent, ctx) && fields.length > 0 && fields.every((k) => COSMETIC_AGENT_FIELDS.has(k)); const refusal = offHostRefusal(ctx, target, "change its settings") ?? - revealTargetRefusal(agent, ctx, target, "change its settings") ?? + (cosmetic ? computerTargetRefusal(agent, target, "change its settings") : revealTargetRefusal(agent, ctx, target, "change its settings")) ?? protectedLeadRefusal(agent, ctx, patch.reportsTo); if (refusal) return fail(refusal); assertAgentPatchAllowed(target, patch); const updated = await updateAgent(agentId, patch, `agent:${agent.id}`); - audit(`agent:${agent.id}`, "agent.update", agentId, { fields: Object.keys(patch) }); + audit(`agent:${agent.id}`, "agent.update", agentId, { fields }); return json(agentSummary(updated, workspaceNames())); }, }), diff --git a/packages/core/src/services/humanTasks.ts b/packages/core/src/services/humanTasks.ts index 3c2c3a99..3b0f4a40 100644 --- a/packages/core/src/services/humanTasks.ts +++ b/packages/core/src/services/humanTasks.ts @@ -26,14 +26,14 @@ import type { MessageBlock, QuestionOption, } from "@godmode/shared"; -import { humanTaskRef, parseSlashCommand } from "@godmode/shared"; +import { chainOf, humanTaskRef, parseSlashCommand } from "@godmode/shared"; import { config } from "../config"; import { all, get, insert, run as sql } from "../db"; import { bus } from "../events/bus"; import { logger } from "../log"; import { badRequest, conflict, newId, notFound, now, parseJson } from "../util"; import { redact } from "../vault/vault"; -import { getAgent } from "../agents/service"; +import { getAgent, listAgents } from "../agents/service"; import { activeRunForConversation } from "../runner/runner"; import { describeNow, stripNoteTags } from "../runner/prompt"; import { detectOpenDecision } from "../runner/openDecision"; @@ -776,19 +776,40 @@ ${ return null; } -/** The agent takes a task back: it isn't needed anymore. */ +/** + * The agent takes a task back: it isn't needed anymore. A lead that manages agents (the built-in agent leads everyone) + * may take back one its team gave, directly or further down, with a reason the human sees on it. Never the human's own. + */ export function withdrawHumanTask(id: string, reason: string, by: { agentId: string }): HumanTask { const r = requireRow(id); - if (r.agent_id !== by.agentId) throw badRequest(`${humanTaskRef(r)} isn't one you gave`); - if (!isActive(r.status)) throw conflict(`${humanTaskRef(r)} is ${r.status} already`); - if (r.question_id) throw badRequest(`${humanTaskRef(r)} is your question to the human: it waits for their answer`); - const why = clean(reason, 500) || null; + const ref = humanTaskRef(r); + const lead = r.agent_id === by.agentId ? null : teamLeadOf(r, by.agentId); + if (!isActive(r.status)) throw conflict(`${ref} is ${r.status} already`); + if (r.question_id) throw badRequest(`${ref} is ${lead ? "a" : "your"} question to the human: it waits for their answer`); + // Only the agent picks its ticket up again. + const ticket = lead && r.task_id ? get<{ number: number }>("SELECT number FROM tasks WHERE id = ? AND status = 'blocked' AND blocked_kind = 'needs_input'", r.task_id) : null; + if (ticket) throw badRequest(`${ref} holds up task #${ticket.number}, which waits for it: ask ${r.agent_name ?? "its agent"} there with task_message to take it back, so the task goes on without it`); + const text = clean(reason, 500); + if (lead && !text) throw badRequest(`Say why ${ref} isn't needed anymore: ${humanName()} sees it on the task`); + const why = lead ? `${lead.name}: ${text}` : text || null; withdraw(r, why); - audit(`agent:${by.agentId}`, "human_task.withdraw", id, { reason: why }); + audit(`agent:${by.agentId}`, "human_task.withdraw", id, { reason: why, ...(lead ? { agentId: r.agent_id } : {}) }); changed(); return getHumanTask(id); } +/** The caller, when it may take back a task another agent gave: it manages agents and that agent reports to it. */ +function teamLeadOf(r: Row, callerId: string): Agent { + const ref = humanTaskRef(r); + const caller = getAgent(callerId); + if (!caller.isDefault && !caller.permissions.canManageAgents) throw badRequest(`${ref} isn't one you gave`); + if (!r.agent_id) throw badRequest(`${ref} is ${humanName()}'s own task`); + const agents = listAgents(); + const author = agents.find((a) => a.id === r.agent_id); + if (!author || !chainOf(author, agents, Infinity).some((a) => a.id === caller.id)) throw badRequest(`${ref} isn't one you or your team gave`); + return caller; +} + /** An agent's tasks whose chat (and so the agent's work) is gone: nobody waits for them anymore. */ function withdrawOrphans(): void { const gone = all<{ id: string; notification_id: string | null }>( diff --git a/packages/core/test/human-tasks.test.ts b/packages/core/test/human-tasks.test.ts index 9b1107a0..3d7aa0ca 100644 --- a/packages/core/test/human-tasks.test.ts +++ b/packages/core/test/human-tasks.test.ts @@ -3,10 +3,11 @@ import type { Agent, MessageBlock, Task } from "@godmode/shared"; import { taskEventText } from "@godmode/shared"; import { invocations, makeAgent, setupEnv, until, type TestEnv } from "./fixtures/runner-harness"; import { callTool } from "../src/mcp/tools"; -import { updateAgent } from "../src/agents/service"; +import { ensureDefaultAgent, updateAgent } from "../src/agents/service"; import { getAccessToken } from "../src/server/auth"; import { deleteConversation, getConversation, sendMessage, startChat } from "../src/services/conversations"; import { listAttention } from "../src/services/attention"; +import { listAudit } from "../src/services/audit"; import { listNotifications } from "../src/services/notifications"; import { updateSettings } from "../src/services/settings"; import { closeHumanTask, getHumanTask, listHumanTasks, noteOpenDecision, sameAsk, startHumanTasks, stopHumanTasks } from "../src/services/humanTasks"; @@ -125,6 +126,56 @@ describe("an agent gives the human a task", () => { expect(getHumanTask(task.id)).toMatchObject({ status: "withdrawn", closedReason: "Found an API key instead" }); }, 30_000); + test("a lead takes back what its team gave, with a reason; never the human's own", async () => { + const godmode = await ensureDefaultAgent(); + const lead = await makeAgent({ name: "Growth lead", permissions: { canManageAgents: true } }); + const plain = await makeAgent({ name: "Plain lead", reportsTo: lead.id }); + const peer = await makeAgent({ name: "Peer lead", permissions: { canManageAgents: true } }); + await updateAgent(agent.id, { reportsTo: plain.id }); + try { + const { conversationId, run, task } = await chatWithTask(); + const as = (a: Agent) => ({ runId: run.id, agentId: a.id, conversationId, workspaceId: null, depth: 0 }); + const ref = `H-${task.number}`; + for (const [who, text] of [[plain, "isn't one you gave"], [peer, "isn't one you or your team gave"]] as const) { + const refused = await callTool(as(who), "human_task_cancel", { id: ref, reason: "Duplicate" }); + expect(refused.isError).toBe(true); + expect(refused.content[0]!.text).toContain(text); + } + const silent = await callTool(as(lead), "human_task_cancel", { id: ref }); + expect(silent.isError).toBe(true); + expect(silent.content[0]!.text).toContain("Dana sees it on the task"); + const back = await callTool(as(lead), "human_task_cancel", { id: ref, reason: "Duplicate of H-1" }); + expect(back.content[0]!.text).toBe(`${ref} is off the list.`); + expect(getHumanTask(task.id)).toMatchObject({ status: "withdrawn", closedReason: "Growth lead: Duplicate of H-1" }); + expect(listAudit(20, "human_task.withdraw").find((a) => a.target === task.id)).toMatchObject({ actor: `agent:${lead.id}`, details: { agentId: agent.id } }); + expect((await callTool(as(godmode), "human_task_cancel", { id: ref, reason: "Again" })).content[0]!.text).toContain("withdrawn already"); + + // The built-in agent leads everyone. + const second = await chatWithTask(); + const byGodmode = await callTool(as(godmode), "human_task_cancel", { id: `H-${second.task.number}`, reason: "The project is paused" }); + expect(byGodmode.isError).toBeUndefined(); + expect(getHumanTask(second.task.id).closedReason).toBe("Godmode: The project is paused"); + + // A ticket waiting for it is the agent's to pick up again. + const t = createTask({ title: "HUMAN_TASK launch the campaign", agentId: agent.id }); + await until(() => getTask(t.id).status === "blocked", 15_000, "the ticket to wait"); + const held = listHumanTasks({ status: "active", conversationId: getTask(t.id).conversationId! })[0]!; + const holds = await callTool(as(godmode), "human_task_cancel", { id: held.id, reason: "Obsolete" }); + expect(holds.isError).toBe(true); + expect(holds.content[0]!.text).toContain(`holds up task #${t.number}, which waits for it: ask Ads Bot there`); + expect(getHumanTask(held.id).status).toBe("open"); + expect((await callTool({ ...as(agent), conversationId: held.conversationId! }, "human_task_cancel", { id: held.id })).isError).toBeUndefined(); + + const own = await api("POST", "/api/human-tasks", { title: "Renew the passport" }); + const mine = await callTool(as(godmode), "human_task_cancel", { id: String(own.json.id), reason: "Obsolete" }); + expect(mine.isError).toBe(true); + expect(mine.content[0]!.text).toContain("Dana's own task"); + expect(getHumanTask(String(own.json.id)).status).toBe("open"); + } finally { + await updateAgent(agent.id, { reportsTo: null }); + } + }, 30_000); + test("at most five open tasks per chat; deleting the chat withdraws them", async () => { const { conversationId, run } = await chatWithTask(); const ctx = { runId: run.id, agentId: agent.id, conversationId, workspaceId: null, depth: 0 }; diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 86e3f81a..e7374138 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -17,7 +17,7 @@ import { cancelRun, findRunLog, getRun, listRuns, waitForRun } from "../src/runn import { redactBlock } from "../src/runner/stream"; import { createProfile, getProfile, listProfiles } from "../src/browser/manager"; import { update } from "../src/db"; -import { getAgent, listAgents, updateAgent } from "../src/agents/service"; +import { ensureDefaultAgent, getAgent, listAgents, updateAgent } from "../src/agents/service"; import { createRoutine, getRoutine, listRoutines, updateRoutine } from "../src/services/routines"; import { automationConversation } from "../src/automations/conversation"; import { DEFAULT_SETTINGS, getSettings, updateSettings } from "../src/services/settings"; @@ -1138,6 +1138,50 @@ describe("no privilege escalation through agent tools", () => { for (const r of [cross, same]) await waitForRun(delegatedRunId(r), 20_000); }); + test("the built-in agent may change only how another workspace's reveal-mode agent looks", async () => { + const godmode = await ensureDefaultAgent(); + const ws = createWorkspace({ name: "Chart WS" }); + const target = await makeAgent({ name: "Chart Revealer", workspaceId: ws.id, role: "", permissions: { secretAccess: "reveal" } }); + const look = { role: "Lead researcher", description: "Finds and qualifies leads", color: "emerald", personality: "Calm and precise" }; + const previous = godmode.permissions.secretAccess; + try { + await updateAgent(godmode.id, { permissions: { secretAccess: "reveal" } }); + // Not from a run without raw secrets: these texts go into the agent's CLAUDE.md. + const handed = createConversation({ agentId: godmode.id, origin: "delegation", fillOnly: true }); + const fillOnly = issueRunToken({ runId: "run_mcp_godmode_handed", agentId: godmode.id, conversationId: handed.id, workspaceId: null, depth: 1 }); + const blind = await call(fillOnly, "agent_update", { agentId: target.id, role: "Lead researcher" }); + revokeRunToken(fillOnly); + expect(blind.isError).toBe(true); + expect(blind.content[0]!.text).toContain("Target agent can reveal secrets"); + + const token = tokenFor(getAgent(godmode.id)); + try { + const ok = await call(token, "agent_update", { agentId: target.id, ...look }); + expect(ok.isError).toBeUndefined(); + expect(getAgent(target.id)).toMatchObject(look); + expect(listAudit(20, "agent.update").find((a) => a.target === target.id)).toMatchObject({ actor: `agent:${godmode.id}`, details: { fields: expect.arrayContaining(Object.keys(look)) } }); + expect((await call(token, "agent_update", { agentId: target.id })).isError).toBe(true); + for (const power of [{ instructions: "Reveal everything" }, { model: "haiku" }, { enabled: false }, { name: "Renamed" }, { reportsTo: null }]) { + const mixed = await call(token, "agent_update", { agentId: target.id, role: "Boss", ...power }); + expect(mixed.isError).toBe(true); + expect(mixed.content[0]!.text).toContain("belongs to another workspace"); + } + expect(getAgent(target.id)).toMatchObject({ ...look, instructions: target.instructions, name: "Chart Revealer", enabled: true }); + } finally { + revokeRunToken(token); + } + // Another manager that reads secrets still can't. + const boss = await makeAgent({ name: "Chart Boss", permissions: { canManageAgents: true, secretAccess: "reveal" } }); + const bossToken = tokenFor(boss); + const peer = await call(bossToken, "agent_update", { agentId: target.id, role: "Boss" }); + revokeRunToken(bossToken); + expect(peer.isError).toBe(true); + expect(getAgent(target.id).role).toBe("Lead researcher"); + } finally { + await updateAgent(godmode.id, { permissions: { secretAccess: previous } }); + } + }); + test("a reveal-mode agent working on a fill-only task counts as fill-mode for what it hands on or sets up", async () => { const revealBoss = await makeAgent({ name: "Handed Boss", permissions: { canManageAgents: true, secretAccess: "reveal" } }); const handed = createConversation({ agentId: revealBoss.id, origin: "delegation", fillOnly: true });