diff --git a/packages/paperclip-runner/scripts/qualify-hermes-runtime.test.mjs b/packages/paperclip-runner/scripts/qualify-hermes-runtime.test.mjs index 4e6ee39d5d..60e9629b42 100644 --- a/packages/paperclip-runner/scripts/qualify-hermes-runtime.test.mjs +++ b/packages/paperclip-runner/scripts/qualify-hermes-runtime.test.mjs @@ -46,14 +46,17 @@ test("pinned Hermes streams through the production ACPX host using a no-auth loc return; } const toolDone = body.messages.slice(lastUser + 1).some(message => message.role === "tool"); - if (!toolDone && (userText.includes("NATIVE_TOOL_FIXTURE") || userText.includes("NATIVE_QUESTION_FIXTURE") || userText.includes("NATIVE_MCP_FIXTURE") || userText.includes("NATIVE_MEMORY_FIXTURE"))) { + if (!toolDone && (userText.includes("NATIVE_TOOL_FIXTURE") || userText.includes("NATIVE_QUESTION_FIXTURE") || userText.includes("NATIVE_MCP_FIXTURE") || userText.includes("NATIVE_MEMORY_FIXTURE") || userText.includes("NATIVE_PLAN_FIXTURE"))) { const question = userText.includes("NATIVE_QUESTION_FIXTURE"); const bridge = userText.includes("NATIVE_MCP_FIXTURE"); const memory = userText.includes("NATIVE_MEMORY_FIXTURE"); - const name = question ? "clarify" : memory ? "memory" : bridge ? body.tools.find(tool => tool.function?.name.endsWith("fixture_probe"))?.function.name ?? "missing_assigned_tool" : "terminal"; + const plan = userText.includes("NATIVE_PLAN_FIXTURE"); + const name = question ? "clarify" : memory ? "memory" : bridge ? body.tools.find(tool => tool.function?.name.endsWith("fixture_probe"))?.function.name ?? "missing_assigned_tool" + : plan ? body.tools.find(tool => tool.function?.name.endsWith("write_document"))?.function.name ?? "missing_plan_tool" : "terminal"; const args = question ? { question: "Choose the fixture color", choices: ["Cobalt", "Amber"] } : memory ? { action: "add", target: "memory", content: "NATIVE_PERSISTED_MEMORY: Project uses Cobalt." } : bridge ? { marker: "assigned-tool-roundtrip" } + : plan ? { marker: "planning-workflow-roundtrip" } : { command: "printf 'native-hermes-command\\n' > command-proof.txt", timeout: 10 }; res.write(`data: ${JSON.stringify({ id: "tool-fixture", object: "chat.completion.chunk", model: "hermes-fixture", created: 1, choices: [{ index: 0, delta: { role: "assistant", tool_calls: [{ index: 0, id: `native-${name}`, type: "function", function: { name, arguments: JSON.stringify(args) } }] }, finish_reason: null }] })}\n\n`); res.end(`data: ${JSON.stringify({ id: "tool-fixture", choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }], usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 } })}\n\ndata: [DONE]\n\n`); @@ -82,7 +85,7 @@ test("pinned Hermes streams through the production ACPX host using a no-auth loc clientCapabilities: acpxProfileClientCapabilities("hermes"), systemInstructions: "This is a transport fixture. Reply briefly.", semanticTools: { - tools: [{ name: "fixture_probe", description: "Return the supplied marker.", inputSchema: { type: "object", properties: { marker: { type: "string" } }, required: ["marker"], additionalProperties: false } }], + tools: ["fixture_probe", "write_document"].map(name => ({ name, description: "Return the supplied marker (simulated semantic authority).", inputSchema: { type: "object", properties: { marker: { type: "string" } }, required: ["marker"], additionalProperties: false } })), handler: async call => { bridgeCalls.push({ tool: call.tool, arguments: call.arguments }); return { marker: "assigned-tool-returned" }; }, }, runtimeContext: { instructions: { workingCopy: { kind: "agent_files", rootPath: agentFiles, entryPath: "AGENTS.md" } }, skills: [], mcp: {} }, @@ -199,4 +202,19 @@ test("pinned Hermes streams through the production ACPX host using a no-auth loc refused = true; } assert.equal(refused, true, "Missing native history was silently replaced with a new conversation"); + await host?.close({ reason: "planning fixture admission" }); + host = await AcpxRuntimeHost.open({ ...options, normalizedSessionId: "hermes-planning-test", providerPolicy: { readOnly: true } }, dependencies); + const planning = host.startTurn({ text: "NATIVE_PLAN_FIXTURE: Use the assigned plan document tool.", requestId: "planning-turn" }); + const planningEvents = []; + for await (const event of planning.events) planningEvents.push(event); + assert.equal((await planning.result).status, "completed"); + assert.deepEqual(bridgeCalls.at(-1), { tool: "write_document", arguments: { marker: "planning-workflow-roundtrip" } }); + assert.ok(planningEvents.some(event => (event.type === "tool_call" || event.type === "tool_call_update") && JSON.stringify(event).includes("assigned-tool-returned")), "Structured native tool output was omitted from the transcript"); + await rm(join(workspace, "command-proof.txt")); + const forbidden = host.startTurn({ text: "NATIVE_TOOL_FIXTURE: Run the supplied command while planning.", requestId: "planning-forbidden-turn" }); + const forbiddenEvents = []; + for await (const event of forbidden.events) forbiddenEvents.push(event); + assert.equal((await forbidden.result).status, "completed"); + assert.equal(await readFile(join(workspace, "command-proof.txt"), "utf8").catch(() => null), null, "Planning mode executed a native write"); + assert.ok(forbiddenEvents.some(event => (event.type === "tool_call" || event.type === "tool_call_update") && JSON.stringify(event).includes("planning mode permits")), `Native policy denial was omitted from the transcript: ${JSON.stringify({ events: forbiddenEvents.filter(event => event.type === "tool_call" || event.type === "tool_call_update"), results: requests.at(-1)?.body.messages.filter(message => message.role === "tool").slice(-1) })}`); }); diff --git a/packages/paperclip-runner/src/drivers/acpx/hermes-installation.ts b/packages/paperclip-runner/src/drivers/acpx/hermes-installation.ts index 9118d1396f..92ff8ddc9b 100644 --- a/packages/paperclip-runner/src/drivers/acpx/hermes-installation.ts +++ b/packages/paperclip-runner/src/drivers/acpx/hermes-installation.ts @@ -8,8 +8,8 @@ import type { QualifiedAcpxProfile } from "./qualified-profiles.js"; // Pins are produced by explicit provisioning and reviewed alongside the bridge. // An unbuilt target has no admission pin; it must not inherit a different ABI. export const HERMES_CLOSURES: Readonly> = Object.freeze({ - "darwin-arm64": "4c89b24335e1869a9faba6996a4e82979337ee5f850d792ab41a838e79afdce3", - "linux-x64": "f9919bd2e812e86e81ecd964d6e1961bb68f96e2154f816c554f31c4f1d78211", + "darwin-arm64": "970f0c48b905d17e616a3b75ef28d91a0e28afba8dbb3218628cd02b3b1e709c", + "linux-x64": "15fc9631318d50a2aafa9c566410b4d486265fb3e58a7981fd0c2525a5fb8f7a", }); export async function verifyHermesInstallation(profile: QualifiedAcpxProfile): Promise { if (profile.agent !== "hermes" || profile.agentServerPackage !== "builtin:hermes-acp" || profile.agentServerVersion !== "1" || profile.agentRuntimePackage !== "native:hermes" || profile.agentRuntimeVersion !== "v2026.9.24") throw new Error("Hermes native profile identity mismatch"); diff --git a/packages/paperclip-runner/src/drivers/acpx/permission-policy.test.ts b/packages/paperclip-runner/src/drivers/acpx/permission-policy.test.ts index 85241d4344..b691acef95 100644 --- a/packages/paperclip-runner/src/drivers/acpx/permission-policy.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/permission-policy.test.ts @@ -4,10 +4,22 @@ import { capabilityCanonicalOperationsForSurface } from "../../catalog/canonical import { acpxRuntimePermissionPolicy, claudePaperclipPermissionRules, + paperclipReadOnlyToolRules, decideAcpxPermission, } from "./permission-policy.js"; describe("ACPX permission policy", () => { + it("admits assigned planning workflow tools without treating them as reads", () => { + const tools = ["write_document", "request_human_input", "set_task_title", "call_api", + "manage_routine", "create_task", "unknown_read", "mcp__other__write_document"] + .map(name => ({ name, annotations: { readOnlyHint: true } })); + expect(paperclipReadOnlyToolRules(tools)).toEqual([ + "mcp__paperclip__call_api", "mcp__paperclip__request_human_input", + "mcp__paperclip__set_task_title", "mcp__paperclip__write_document", + ]); + expect(claudePaperclipPermissionRules(tools, "approve-reads")).toEqual([]); + expect(paperclipReadOnlyToolRules([])).toEqual([]); + }); it.each(["approve-reads", "approve-paperclip"] as const)("allows assigned canonical live reads, including approval lookup, in %s", mode => { const reads = capabilityCanonicalOperationsForSurface("live").filter(action => action.sideEffectClass === "read"); expect(reads.some(action => action.operationId === "get_approval")).toBe(true); diff --git a/packages/paperclip-runner/src/drivers/acpx/permission-policy.ts b/packages/paperclip-runner/src/drivers/acpx/permission-policy.ts index 63076db5ea..d0f7e2841a 100644 --- a/packages/paperclip-runner/src/drivers/acpx/permission-policy.ts +++ b/packages/paperclip-runner/src/drivers/acpx/permission-policy.ts @@ -27,6 +27,22 @@ const AUTOMATIC_PAPERCLIP_WORKFLOW_ACTIONS = new Set([ "create_project", "request_approval", ]); +/** Assigned control-plane tools permitted in a read-only task mode. + * This exempts them from the native filesystem-write guard, not the active + * task's semantic authorization or the configured permission prompt. + */ +export function paperclipReadOnlyToolRules( + tools: readonly Readonly>[], +): string[] { + return [...new Set(tools.flatMap(tool => { + if (typeof tool.name !== "string") return []; + const name = canonicalRunnerToolName(tool.name); + const action = capabilityCanonicalOperation(name); + return action?.surfaces.includes("live") && action.taskModes.some(mode => mode === "planning" || mode === "ask") + ? [`mcp__paperclip__${name}`] : []; + }))].sort(); +} + /** Exact SDK rules for the run's runner-owned Paperclip MCP connection. */ export function claudePaperclipPermissionRules( tools: readonly Readonly>[], diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts index d3e70fe721..e6443e77f9 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts @@ -30,7 +30,7 @@ import { import { createSanitizedAcpxSpawnInput } from "./environment.js"; import { cursorInstructionBinding } from "./cursor-instructions.js"; import { parseHermesConfig } from "./hermes-config.js"; -import { claudePaperclipPermissionRules } from "./permission-policy.js"; +import { claudePaperclipPermissionRules, paperclipReadOnlyToolRules } from "./permission-policy.js"; import type { QualifiedAcpxAgent } from "./qualified-profiles.js"; import { resolveAcpxRuntimeRoot, @@ -506,6 +506,7 @@ export async function prepareAcpxRuntimeSandbox(input: { PAPERCLIP_HERMES_SYSTEM_INSTRUCTIONS: policy.systemInstructions, PAPERCLIP_HERMES_POLICY: JSON.stringify({ readOnly: policy.readOnly, readRoots: policy.readRoots, protectedPaths, permissionMode: input.binding.permissionMode, paperclipReadTools: claudePaperclipPermissionRules(input.tools ?? [], "approve-reads"), + paperclipReadOnlyTools: paperclipReadOnlyToolRules(input.tools ?? []), paperclipAutomaticTools: claudePaperclipPermissionRules(input.tools ?? [], input.binding.permissionMode), }), } : {}), diff --git a/packages/paperclip-runner/src/providers/hermes/bridge.py b/packages/paperclip-runner/src/providers/hermes/bridge.py index b892dd8272..1bef70e2fe 100644 --- a/packages/paperclip-runner/src/providers/hermes/bridge.py +++ b/packages/paperclip-runner/src/providers/hermes/bridge.py @@ -248,6 +248,11 @@ class ManagedHermesACPAgent(HermesACPAgent): async def _replay_session_history(self, state): if not self._conn: raise ValueError("Hermes history replay has no client") + if self._negotiated: + # Paperclip restores its durable transcript independently. Native + # history has already been strictly loaded into the agent; replay + # notifications must not be mistaken for this turn's live output. + return for update in _history_replay_updates(state.history): await self._conn.session_update(session_id=state.session_id, update=update) @@ -364,6 +369,7 @@ class ManagedHermesACPAgent(HermesACPAgent): # Native start/complete callbacks carry the actual call ID, including # overlapping invocations and out-of-order completion of the same tool. calls, metadata = callbacks.tool_call_ids, callbacks.tool_call_meta + completed_denials = set() turn_state = {"saw_completion": True} def tool_start(call_id, name, args): @@ -376,16 +382,31 @@ class ManagedHermesACPAgent(HermesACPAgent): _send_update(conn, session_id, loop, build_tool_start(call_id, name, args)) def tool_complete(call_id, name, args, result): + if call_id in completed_denials: + return if call_id not in metadata: raise ValueError("Hermes completed an unknown tool call") meta = metadata.pop(call_id) calls[name].remove(call_id) if not calls[name]: calls.pop(name) - _send_update(conn, session_id, loop, build_tool_complete( - call_id, name, result=result if isinstance(result, str) else json.dumps(result), - function_args=args, snapshot=meta["snapshot"], - )) + result_text = result if isinstance(result, str) else json.dumps(result) + try: + envelope = json.loads(result_text) + except (ValueError, TypeError): + envelope = None + # Native policy and MCP transport failures use this exact envelope; + # an arbitrary successful payload mentioning errors is not a failure. + is_error = (isinstance(envelope, dict) and set(envelope) == {"error"} + and isinstance(envelope["error"], str) and bool(envelope["error"])) + update = build_tool_complete( + call_id, name, result=result_text, + function_args=args, snapshot=meta["snapshot"], is_error=is_error, + ) + # Hermes deliberately omits raw output for its polished/structured + # cards. ACPX consumes rawOutput for the runner transcript; keep + # native content (including diffs) and the original result together. + _send_update(conn, session_id, loop, update.model_copy(update={"raw_output": result_text})) def progress(event, name=None, preview=None, args=None, **details): child = details.get("child_session_id") or details.get("subagent_id") @@ -416,10 +437,10 @@ class ManagedHermesACPAgent(HermesACPAgent): # Both sequential inline tools and concurrent registry tools # cross this native execution boundary, including child agents. def checked(final_args): - return authorized(name, final_args, execute) + return authorized(name, final_args, execute, kwargs.get("tool_call_id")) return self._dispatch_original(name, args, checked, **kwargs) - def authorized(name, args, execute): + def authorized(name, args, execute, call_id): try: read = authorize_tool(name, args, policy=policy, cwd=state.cwd, assigned_skills=assigned) if self._active != (session_id, token) or state.cancel_event.is_set(): @@ -436,7 +457,17 @@ class ManagedHermesACPAgent(HermesACPAgent): finally: tool_policy.reset(scope) except PermissionError as error: - return json.dumps({"error": str(error)}) + result = json.dumps({"error": str(error)}) + # Policy is evaluated before Hermes's dispatch/start hook. + # Its executor can therefore return a denied result without + # emitting a start, and its later complete hook is too late + # to pair it. Project the denial using the native hook ID. + if isinstance(call_id, str) and call_id: + if call_id not in metadata: + tool_start(call_id, name, args) + tool_complete(call_id, name, args, result) + completed_denials.add(call_id) + return result middleware.run_tool_execution_middleware = dispatch def clarify(question=None, choices=None, multi_select=False, questions=None): diff --git a/packages/paperclip-runner/src/providers/hermes/policy.py b/packages/paperclip-runner/src/providers/hermes/policy.py index 488b858571..fab3de8c79 100644 --- a/packages/paperclip-runner/src/providers/hermes/policy.py +++ b/packages/paperclip-runner/src/providers/hermes/policy.py @@ -19,10 +19,10 @@ def authorize_tool(name, args, *, policy, cwd, assigned_skills=()): read = (name in READ_TOOLS or name in REPORTING_TOOLS or name in policy.get("paperclipReadTools", []) or (name == "memory" and args.get("action") in {"read", "view"})) - if policy["readOnly"] and not read: + if policy["readOnly"] and not read and name not in policy.get("paperclipReadOnlyTools", []): # Arbitrary commands, code execution and delegation can write, even when # their natural-language description claims they only inspect state. - raise PermissionError("Paperclip planning mode permits only read tools") + raise PermissionError("Paperclip planning mode permits only native reads and authorized Paperclip workflow tools") protected = [Path(p).resolve() for p in policy.get("protectedPaths", [])] assigned = [Path(p).resolve() for p in assigned_skills] diff --git a/packages/paperclip-runner/src/providers/hermes/test_bridge.py b/packages/paperclip-runner/src/providers/hermes/test_bridge.py index e9b5063eeb..93821c4299 100644 --- a/packages/paperclip-runner/src/providers/hermes/test_bridge.py +++ b/packages/paperclip-runner/src/providers/hermes/test_bridge.py @@ -26,6 +26,17 @@ class Accounting(unittest.TestCase): class ToolPolicy(unittest.TestCase): + def test_planning_workflow_mutations_reach_semantic_authority_but_are_not_reads(self): + assigned = ["mcp__paperclip__write_document", "mcp__paperclip__request_human_input", "mcp__paperclip__set_task_title", "mcp__paperclip__call_api"] + policy = {"permissionMode": "approve-all", "readOnly": True, "paperclipReadOnlyTools": assigned} + for name in assigned: + self.assertFalse(authorize_tool(name, {}, policy=policy, cwd="/workspace")) + for name in ["terminal", "write_file", "mcp__other__write_document", "mcp__paperclip__manage_routine"]: + with self.assertRaises(PermissionError): + authorize_tool(name, {}, policy=policy, cwd="/workspace") + with self.assertRaises(PermissionError): + authorize_tool(assigned[0], {}, policy={**policy, "permissionMode": "deny-all"}, cwd="/workspace") + def test_planning_can_report_and_use_only_authoritative_assigned_reads(self): policy = {"permissionMode": "approve-reads", "readOnly": True, "paperclipReadTools": ["mcp__paperclip__read_document"]} @@ -130,7 +141,7 @@ class Restore(unittest.TestCase): class Controls(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self): self.state = SimpleNamespace(runtime_lock=threading.Lock(), is_running=True, - cancel_event=threading.Event(), agent=Mock()) + cancel_event=threading.Event(), agent=Mock(), cwd="/workspace") self.manager = Mock() self.manager.get_session.return_value = self.state self.bridge = ManagedHermesACPAgent(self.manager) @@ -173,6 +184,47 @@ class Controls(unittest.IsolatedAsyncioTestCase): self.assertEqual(callbacks.tool_call_ids, {}) self.assertEqual(callbacks.tool_call_meta, {}) + async def test_structured_tool_results_retain_native_content_and_raw_output(self): + callbacks = SimpleNamespace(tool_call_ids={}, tool_call_meta={}) + updates = [] + with patch("bridge.HermesACPAgent._wire_turn_callbacks", return_value=callbacks), patch("bridge._send_update", side_effect=lambda c, s, l, event: updates.append(event)): + self.bridge._wire_turn_callbacks(self.state, "session", Mock(), asyncio.get_running_loop()) + for call_id, result in [("success", {"marker": "native-result"}), ("failure", {"error": "denied"})]: + self.state.agent.tool_start_callback(call_id, "mcp__paperclip__write_document", {}) + self.state.agent.tool_complete_callback(call_id, "mcp__paperclip__write_document", {}, result) + self.assertEqual(json.loads(updates[1].raw_output), {"marker": "native-result"}) + self.assertTrue(updates[1].content) + self.assertEqual(updates[3].status, "failed") + self.assertEqual(json.loads(updates[3].raw_output), {"error": "denied"}) + + async def test_pre_dispatch_denials_keep_native_ids_and_complete_exactly_once(self): + from hermes_cli import middleware + callbacks = SimpleNamespace(tool_call_ids={}, tool_call_meta={}) + updates = [] + execute = Mock() + with patch.dict("os.environ", {"PAPERCLIP_HERMES_POLICY": json.dumps({"permissionMode": "approve-all", "readOnly": True})}), patch("hermes_cli.middleware.run_tool_execution_middleware", side_effect=lambda name, args, run, **kwargs: run(args)), patch("bridge.HermesACPAgent._wire_turn_callbacks", return_value=callbacks), patch("bridge._send_update", side_effect=lambda c, s, l, event: updates.append(event)): + self.bridge._wire_turn_callbacks(self.state, "session", Mock(), asyncio.get_running_loop()) + for call_id in ["denial-a", "denial-b"]: + result = middleware.run_tool_execution_middleware("terminal", {"command": "touch proof"}, execute, tool_call_id=call_id) + self.state.agent.tool_complete_callback(call_id, "terminal", {}, result) + execute.assert_not_called() + self.assertEqual([update.tool_call_id for update in updates], ["denial-a", "denial-a", "denial-b", "denial-b"]) + for update in updates[1::2]: + self.assertEqual(update.status, "failed") + self.assertIn("planning mode permits", update.raw_output) + self.assertEqual(callbacks.tool_call_meta, {}) + + async def test_managed_restore_retains_history_without_replaying_it_as_live_output(self): + self.bridge._conn = SimpleNamespace(session_update=AsyncMock()) + history = [{"role": "assistant", "content": "Previous reply"}] + state = SimpleNamespace(session_id="session", history=history) + await self.bridge._replay_session_history(state) + self.bridge._conn.session_update.assert_not_called() + self.assertEqual(state.history, history) + self.bridge._negotiated = False + await self.bridge._replay_session_history(state) + self.bridge._conn.session_update.assert_awaited_once() + async def test_capabilities_use_native_sdk_metadata(self): response = await self.bridge.initialize(client_capabilities=ClientCapabilities( field_meta={"paperclipHermes": {"version": 1}}))