P2: normalize tool args + remove tool.use#974
Merged
Conversation
magicpro97
commented
Jun 9, 2026
magicpro97
commented
Owner
- Add normalizeToolArgs(): name->skill, oldString->old_str, newString->new_str, content->file_text, path->filePath
- Apply in tool.execute.before/after
- Remove tool.use (undocumented hook, dead code)
- 5 new tests
- Unlocks 6 rules: SkillUsageRule, SyntaxGateRule, FileSizeAdvisoryRule, BlockUnsafeHtmlRule, AutoBugDetectorRule, TokenTrackerRule
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.
Owner
Author
|
/copilot-review |
Owner
Author
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenCode → Copilot hooks bridge to normalize tool argument field names before invoking hook_runner.py, and removes the unused/undocumented tool.use hook to align with the intended event surface.
Changes:
- Add
normalizeToolArgs()to backfill legacy → canonical arg keys (e.g.,name→skill,oldString/newString→old_str/new_str,content→file_text,path→filePath). - Apply normalization to both
tool.execute.beforeandtool.execute.afterhook payloads, includingtoolResult.filePathhandling. - Remove the
tool.usehook handler and add/adjust tests to reflect the new bridge contract.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
opencode-plugin/copilot-tools-bridge.ts |
Adds arg normalization, applies it to before/after events, and removes the tool.use hook handler. |
tests/test_opencode_bridge.py |
Updates plugin-export expectations and adds compatibility tests for normalized arg shapes. |
Comment on lines
+73
to
+89
| function normalizeToolArgs(toolName: string, args: Record<string, unknown>): Record<string, unknown> { | ||
| 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 | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed. Added typeof canonicalField !== 'string' guard on all mappings so already-set values are never overwritten. The filePath mapping already had this guard (was using !normalized.filePath — changed to typeof for consistency). Committed in f6dd643.
Comment on lines
+356
to
+366
| 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}") |
Owner
Author
There was a problem hiding this comment.
Fair. Renamed to test_skill_without_normalized_skill_field with matching error message. Committed in f6dd643.
- 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)
Owner
Author
Copilot stopped work on behalf of
magicpro97 due to an error
June 9, 2026 06:16
Copilot stopped work on behalf of
magicpro97 due to an error
June 9, 2026 06:16
Owner
Author
|
@copilot review please |
Copilot stopped work on behalf of
magicpro97 due to an error
June 9, 2026 06:17
This was referenced Jun 9, 2026
magicpro97
added a commit
that referenced
this pull request
Jun 9, 2026
…n, cwd, stdout, shell.env ## Summary Closes all remaining data gaps between the opencode bridge and hook_runner rules. Part of Phase 2 opencode bridge (continuing from PR #973, #974, #975). ### Changes 1. **exitCode/exit_code/stdout in toolResult** — unlocks SkillUsageRule, TokenTrackerRule, VerificationGateRule 2. **reason in sessionEnd** — idle events pass reason="idle" instead of "unknown" 3. **cwd in userPromptSubmitted** — UserPromptContextRule finds CONTEXT.md correctly 4. **shell.env hook** — injects COPILOT_AGENT_SESSION_ID into all subprocess environments ### Testing 39/39 bridge tests pass, 13/13 security tests pass. ### Phase 2 Complete Status - ~90% rules compatible with opencode - 12 bridge hooks covering all 10 hook_runner events - Remaining gaps (agentStop, voting) are CLI-only or semantically inappropriate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.