From a79e6b0dbebe0a96698e852e1774c401b0db5d30 Mon Sep 17 00:00:00 2001 From: Linh Ngo Date: Tue, 9 Jun 2026 13:10:20 +0700 Subject: [PATCH 1/2] feat(opencode-bridge): normalize tool args and remove dead tool.use hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2 changes: - Add normalizeToolArgs(): maps opencode field names to Copilot CLI field names so rules can read the correct keys • skill: name → skill (fixes SkillUsageRule) • edit: oldString/newString → old_str/new_str (fixes SyntaxGateRule, FileSizeAdvisoryRule, BlockUnsafeHtmlRule, AutoBugDetectorRule, TokenTrackerRule) • write (→create): content → file_text (fixes FileSizeAdvisoryRule, BlockUnsafeHtmlRule, SyntaxGateRule, AutoBugDetectorRule, TokenTrackerRule) • all tools: path → filePath when no filePath set - Apply normalizeToolArgs in tool.execute.before and tool.execute.after - Remove tool.use handler (undocumented in opencode plugin API, dead code) - Add 5 new tests: 1 source-level + 4 hook runner integration tests - Remove tool.use assertion from test_plugin_has_required_exports Unlocks 6 rules: SkillUsageRule, SyntaxGateRule, FileSizeAdvisoryRule, BlockUnsafeHtmlRule, AutoBugDetectorRule, TokenTrackerRule. --- opencode-plugin/copilot-tools-bridge.ts | 53 ++++++++-------- tests/test_opencode_bridge.py | 83 ++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 28 deletions(-) diff --git a/opencode-plugin/copilot-tools-bridge.ts b/opencode-plugin/copilot-tools-bridge.ts index eccbd9b0..5de9978d 100644 --- a/opencode-plugin/copilot-tools-bridge.ts +++ b/opencode-plugin/copilot-tools-bridge.ts @@ -70,6 +70,24 @@ function mapToolName(tool: string): string { return tool } +function normalizeToolArgs(toolName: string, args: Record): Record { + const normalized = { ...args } + if (toolName === "skill" && typeof normalized.name === "string") { + normalized.skill = normalized.name + } + if (toolName === "edit") { + if (typeof normalized.oldString === "string") normalized.old_str = normalized.oldString + if (typeof normalized.newString === "string") normalized.new_str = normalized.newString + } + if (toolName === "create") { + if (typeof normalized.content === "string") normalized.file_text = normalized.content + } + if (typeof normalized.path === "string" && !normalized.filePath) { + normalized.filePath = normalized.path + } + return normalized +} + type HookState = { sessionStartFired: boolean sessionId: string @@ -105,11 +123,12 @@ export const CopilotToolsBridge: Plugin = async ({ project, client, $, directory "tool.execute.before": async (input, output) => { const toolName = mapToolName(input.tool) + const toolArgs = normalizeToolArgs(toolName, output.args ?? {}) const results = await callHookRunner($, client, "preToolUse", { toolName, - toolArgs: output.args, - toolInput: output.args, + toolArgs, + toolInput: toolArgs, sessionId: input.sessionID, callId: input.callID, cwd: process.cwd(), @@ -126,20 +145,21 @@ export const CopilotToolsBridge: Plugin = async ({ project, client, $, directory "tool.execute.after": async (input, output) => { const toolName = mapToolName(input.tool) + const toolArgs = normalizeToolArgs(toolName, (input.args ?? {}) as Record) const toolResult: Record = { title: output.title, output: output.output, resultType: output.isError ? "error" : "success", } - if (typeof input.args === "object" && input.args && (input.args as any).filePath) { - toolResult.filePath = (input.args as any).filePath + if (typeof toolArgs.filePath === "string") { + toolResult.filePath = toolArgs.filePath } const results = await callHookRunner($, client, "postToolUse", { toolName, - toolArgs: input.args, - toolInput: input.args, + toolArgs, + toolInput: toolArgs, toolResult, sessionId: input.sessionID, cwd: process.cwd(), @@ -155,27 +175,6 @@ export const CopilotToolsBridge: Plugin = async ({ project, client, $, directory } }, - "tool.use": async (input, output) => { - const toolName = mapToolName(input.tool) - - const results = await callHookRunner($, client, "preToolUse", { - toolName, - toolArgs: output.args, - toolInput: output.args, - sessionId: input.sessionID, - callId: input.callID, - cwd: process.cwd(), - }) - - if (!results) return - - for (const parsed of results) { - if (parsed.permissionDecision === "deny") { - throw new Error(parsed.permissionDecisionReason || `Blocked by ${toolName} rule`) - } - } - }, - "chat.message": async (input, output) => { if (!state.sessionStartFired) { await fireSessionStart(input.sessionID) diff --git a/tests/test_opencode_bridge.py b/tests/test_opencode_bridge.py index b72f4250..0ea24918 100644 --- a/tests/test_opencode_bridge.py +++ b/tests/test_opencode_bridge.py @@ -83,7 +83,7 @@ def test_plugin_has_required_exports(self): text = PLUGIN_SRC.read_text(encoding="utf-8") self.assertIn("tool.execute.before", text) self.assertIn("tool.execute.after", text) - self.assertIn("tool.use", text) + self.assertNotIn("tool.use", text) # removed in P2 (undocumented, dead code) self.assertIn("task", text) self.assertIn("chat.message", text) self.assertIn("CopilotToolsBridge", text) @@ -100,6 +100,15 @@ def test_plugin_maps_tool_names(self): self.assertIn('return "view"', text) self.assertIn('return "edit"', text) + def test_plugin_normalizes_tool_args(self): + text = PLUGIN_SRC.read_text(encoding="utf-8") + self.assertIn("normalizeToolArgs", text) + self.assertIn("normalized.skill = normalized.name", text) + self.assertIn("normalized.old_str = normalized.oldString", text) + self.assertIn("normalized.new_str = normalized.newString", text) + self.assertIn("normalized.file_text = normalized.content", text) + self.assertIn("normalized.filePath = normalized.path", text) + class TestInstallSandboxed(unittest.TestCase): """Install tests run against a temp XDG_CONFIG_HOME sandbox with isolated HOME.""" @@ -332,6 +341,78 @@ def test_user_prompt_submitted(self): ) self.assertEqual(proc.returncode, 0, f"userPromptSubmitted failed:\n{proc.stderr}") + def test_skill_normalized_args(self): + proc = _run_hook( + "postToolUse", + { + "toolName": "skill", + "toolArgs": {"name": "frontend-dev", "skill": "frontend-dev"}, + "toolInput": {"name": "frontend-dev", "skill": "frontend-dev"}, + "sessionId": "bridge-test-skill-001", + }, + ) + self.assertEqual(proc.returncode, 0, f"skill postToolUse failed:\n{proc.stderr}") + + def test_skill_normalized_args_original_field_preserved(self): + proc = _run_hook( + "postToolUse", + { + "toolName": "skill", + "toolArgs": {"name": "test-skill"}, + "toolInput": {"name": "test-skill"}, + "sessionId": "bridge-test-skill-002", + }, + ) + self.assertEqual(proc.returncode, 0, f"skill without normalized 'skill' field:\n{proc.stderr}") + + def test_edit_normalized_args(self): + proc = _run_hook( + "preToolUse", + { + "toolName": "edit", + "toolArgs": { + "path": os.path.join(tempfile.gettempdir(), "bridge-test-edit.py"), + "filePath": os.path.join(tempfile.gettempdir(), "bridge-test-edit.py"), + "oldString": "foo", + "newString": "bar", + "old_str": "foo", + "new_str": "bar", + }, + "toolInput": { + "path": os.path.join(tempfile.gettempdir(), "bridge-test-edit.py"), + "filePath": os.path.join(tempfile.gettempdir(), "bridge-test-edit.py"), + "oldString": "foo", + "newString": "bar", + "old_str": "foo", + "new_str": "bar", + }, + "sessionId": "bridge-test-edit-001", + }, + ) + self.assertEqual(proc.returncode, 0, f"edit preToolUse failed:\n{proc.stderr}") + + def test_write_normalized_args(self): + proc = _run_hook( + "preToolUse", + { + "toolName": "create", + "toolArgs": { + "path": os.path.join(tempfile.gettempdir(), "bridge-test-write.py"), + "filePath": os.path.join(tempfile.gettempdir(), "bridge-test-write.py"), + "content": "print('hello')", + "file_text": "print('hello')", + }, + "toolInput": { + "path": os.path.join(tempfile.gettempdir(), "bridge-test-write.py"), + "filePath": os.path.join(tempfile.gettempdir(), "bridge-test-write.py"), + "content": "print('hello')", + "file_text": "print('hello')", + }, + "sessionId": "bridge-test-write-001", + }, + ) + self.assertEqual(proc.returncode, 0, f"create preToolUse failed:\n{proc.stderr}") + class TestMCPConfig(unittest.TestCase): """Verify MCP server file exists in repo.""" From f6dd6433d0ae8644dfa676c0f9ee462cfdf1896a Mon Sep 17 00:00:00 2001 From: Linh Ngo Date: Tue, 9 Jun 2026 13:15:03 +0700 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20copilot=20review=20?= =?UTF-8?q?=E2=80=94=20guard=20canonical=20fields,=20rename=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - normalizeToolArgs: only set canonical fields when not already present (typeof check prevents overwriting intentionally-set values) - Rename test_skill_normalized_args_original_field_preserved → test_skill_without_normalized_skill_field (self-documenting intent) --- opencode-plugin/copilot-tools-bridge.ts | 10 +++++----- tests/test_opencode_bridge.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/opencode-plugin/copilot-tools-bridge.ts b/opencode-plugin/copilot-tools-bridge.ts index 5de9978d..a86d3f84 100644 --- a/opencode-plugin/copilot-tools-bridge.ts +++ b/opencode-plugin/copilot-tools-bridge.ts @@ -72,17 +72,17 @@ function mapToolName(tool: string): string { function normalizeToolArgs(toolName: string, args: Record): Record { const normalized = { ...args } - if (toolName === "skill" && typeof normalized.name === "string") { + if (toolName === "skill" && typeof normalized.name === "string" && typeof normalized.skill !== "string") { normalized.skill = normalized.name } if (toolName === "edit") { - if (typeof normalized.oldString === "string") normalized.old_str = normalized.oldString - if (typeof normalized.newString === "string") normalized.new_str = normalized.newString + if (typeof normalized.oldString === "string" && typeof normalized.old_str !== "string") normalized.old_str = normalized.oldString + if (typeof normalized.newString === "string" && typeof normalized.new_str !== "string") normalized.new_str = normalized.newString } if (toolName === "create") { - if (typeof normalized.content === "string") normalized.file_text = normalized.content + if (typeof normalized.content === "string" && typeof normalized.file_text !== "string") normalized.file_text = normalized.content } - if (typeof normalized.path === "string" && !normalized.filePath) { + if (typeof normalized.path === "string" && typeof normalized.filePath !== "string") { normalized.filePath = normalized.path } return normalized diff --git a/tests/test_opencode_bridge.py b/tests/test_opencode_bridge.py index 0ea24918..70f9aa69 100644 --- a/tests/test_opencode_bridge.py +++ b/tests/test_opencode_bridge.py @@ -353,7 +353,7 @@ def test_skill_normalized_args(self): ) self.assertEqual(proc.returncode, 0, f"skill postToolUse failed:\n{proc.stderr}") - def test_skill_normalized_args_original_field_preserved(self): + def test_skill_without_normalized_skill_field(self): proc = _run_hook( "postToolUse", { @@ -363,7 +363,7 @@ def test_skill_normalized_args_original_field_preserved(self): "sessionId": "bridge-test-skill-002", }, ) - self.assertEqual(proc.returncode, 0, f"skill without normalized 'skill' field:\n{proc.stderr}") + self.assertEqual(proc.returncode, 0, f"skill postToolUse without 'skill' field:\n{proc.stderr}") def test_edit_normalized_args(self): proc = _run_hook(