mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-11 23:36:51 +02:00
fix(hermes): preserve planning authority and native transcript results
Qualification exposed denied plan workflows, missing structured tool output, and cancellation replay in subsequent turns. Admit assigned control-plane operations through their existing semantic authority, project denied native calls by identity, and keep managed history restoration separate from transcript replay. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
45a624a5fb
commit
9bb60ab1b8
8 files changed
+146
-16
No files matched your search
@@ -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) })}`);
|
||||
});
|
||||
@@ -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<Record<string, string>> = Object.freeze({
|
||||
"darwin-arm64": "4c89b24335e1869a9faba6996a4e82979337ee5f850d792ab41a838e79afdce3",
|
||||
"linux-x64": "f9919bd2e812e86e81ecd964d6e1961bb68f96e2154f816c554f31c4f1d78211",
|
||||
"darwin-arm64": "970f0c48b905d17e616a3b75ef28d91a0e28afba8dbb3218628cd02b3b1e709c",
|
||||
"linux-x64": "15fc9631318d50a2aafa9c566410b4d486265fb3e58a7981fd0c2525a5fb8f7a",
|
||||
});
|
||||
export async function verifyHermesInstallation(profile: QualifiedAcpxProfile): Promise<VerifiedAcpxInstallation> {
|
||||
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");
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<Record<string, unknown>>[],
|
||||
): 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<Record<string, unknown>>[],
|
||||
|
||||
@@ -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),
|
||||
}),
|
||||
} : {}),
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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]
|
||||
|
||||
|
||||
@@ -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}}))
|
||||
|
||||
Reference in new issue
Block a user