feat: update Kilo targets to reflect Kilo CLI rebase onto OpenCode (#1411)#1413
Conversation
|
Thank you for your contribution! Unfortunately, this PR has 5096 added lines, which exceeds the limit of 1000 lines for external contributors. Please split your changes into smaller PRs. See CONTRIBUTING.md for details. |
There was a problem hiding this comment.
Pull request overview
Updates the kilo target implementation to align with Kilo’s new (OpenCode-based) internal architecture, including updated file locations, format mappings, and new tool feature support while keeping .kilo / .config/kilo structures.
Changes:
- Reworks Kilo processors for rules/skills/commands/MCP to match the OpenCode-style generation and path conventions.
- Adds Kilo support for hooks and subagents (including event-name mapping and processors wiring).
- Expands/updates test coverage for the new Kilo behavior across features.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| update-kilo-skill.py | Helper script to generate Kilo skill files from OpenCode equivalents. |
| update-kilo-rule.py | Helper script to generate Kilo rule files from OpenCode equivalents. |
| update-kilo-mcp.py | Helper script to generate Kilo MCP files from OpenCode equivalents. |
| update-kilo-command.py | Helper script to generate Kilo command files from OpenCode equivalents. |
| src/types/hooks.ts | Adds Kilo hook events, config schema support, and canonical→Kilo event mapping. |
| src/features/subagents/subagents-processor.ts | Registers kilo as a supported subagent tool target/factory. |
| src/features/subagents/subagents-processor.test.ts | Updates expectations to include kilo in subagent target lists. |
| src/features/subagents/kilo-subagent.ts | Implements Kilo subagent read/write + Rulesync conversion. |
| src/features/subagents/kilo-subagent.test.ts | Adds tests for Kilo subagent paths, conversion, and parsing. |
| src/features/skills/rulesync-skill.ts | Extends Rulesync skill frontmatter typing to include kilo.allowed-tools. |
| src/features/skills/kilo-skill.ts | Updates Kilo skill paths/format and maps allowed-tools into Rulesync metadata. |
| src/features/skills/kilo-skill.test.ts | Updates/expands Kilo skill tests for new paths + allowed-tools mapping. |
| src/features/rules/kilo-rule.ts | Updates Kilo rules to AgentsMd-style AGENTS.md root + .kilo/rules non-root handling. |
| src/features/rules/kilo-rule.test.ts | Major test updates for root/non-root rules, global mode, and agentsmd subprojectPath behavior. |
| src/features/mcp/kilo-mcp.ts | Reworks Kilo MCP to use kilo.json{c} and converts between Kilo-native and standard MCP formats. |
| src/features/mcp/kilo-mcp.test.ts | Adds extensive tests for Kilo MCP JSON/JSONC preference, conversions, tools mapping, and global mode. |
| src/features/hooks/kilo-hooks.ts | Adds Kilo hooks plugin generator and wiring to Rulesync hooks config. |
| src/features/hooks/kilo-hooks.test.ts | Adds tests for Kilo hook filtering, matcher handling, escaping, and pathing. |
| src/features/hooks/hooks-processor.ts | Registers kilo in hooks processor targets/factories and declares supported events/types. |
| src/features/hooks/hooks-processor.test.ts | Updates hooks processor target expectations to include kilo. |
| src/features/commands/kilo-command.ts | Reworks Kilo commands to match OpenCode-style frontmatter/body handling and .kilo/.config/kilo paths. |
| src/features/commands/kilo-command.test.ts | Updates tests for new Kilo command paths, frontmatter schema, conversion, and file parsing. |
| patch-subagents-processor.py | Helper script to patch subagents processor registration for Kilo. |
| patch-subagents-processor-tuple.py | Helper script to patch subagents processor tool-target tuple for Kilo. |
| patch-rulesync-skill.py | Helper script to patch Rulesync skill schema/type to add Kilo section. |
| patch-hooks-types.py | Helper script to patch hooks types with Kilo events and mappings. |
| patch-hooks-processor.py | Helper script to patch hooks processor registration for Kilo. |
| fix-hooks-processor.py | Helper script to fix a prior hooks-processor patch string issue. |
| static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { | ||
| if (global) { | ||
| return { | ||
| relativeDirPath: join(".config", "kilo"), | ||
| relativeFilePath: "kilo.json", | ||
| }; | ||
| } | ||
| return { | ||
| relativeDirPath: ".kilocode", | ||
| relativeFilePath: "mcp.json", | ||
| relativeDirPath: ".", | ||
| relativeFilePath: "kilo.json", | ||
| }; |
There was a problem hiding this comment.
PR description/issue #1411 calls out updating Kilo .gitignore entries away from legacy .kilocode/* paths, but the repo still has Kilo entries pointing at .kilocode/... (e.g. src/cli/commands/gitignore-entries.ts:154-158) and .kilocodeignore. This likely means rulesync gitignore will keep emitting outdated patterns even after these path changes; please update the gitignore entries (and related tests) to match the new .kilo/** / .config/kilo/** locations and kilo.json{c}.
update-kilo-rule.py
Outdated
| import re | ||
|
|
||
| with open("src/features/rules/opencode-rule.ts", "r") as f: | ||
| content = f.read() | ||
|
|
||
| content = content.replace("OpenCode", "Kilo") | ||
| content = content.replace("opencode", "kilo") | ||
| content = content.replace(".config/kilo", ".config/kilo") | ||
| content = content.replace(".kilo\", \"memories\"", ".kilo\", \"rules\"") | ||
| content = content.replace("opencode-rule", "kilo-rule") |
There was a problem hiding this comment.
This script has an unused re import, and the .replace(".config/kilo", ".config/kilo") lines are no-ops. Consider removing the unused import and fixing/removing the no-op replacements to avoid keeping misleading or broken maintenance scripts in the repo.
dyoshikawa
left a comment
dyoshikawa
left a comment
There was a problem hiding this comment.
This PR makes significant changes to align Kilo support with the OpenCode rebase. The new hooks and subagents support is well-structured, and the directory migration looks correct overall. A few things stand out that should be addressed before merging:
The Python scaffolding scripts (fix-hooks-processor.py, patch-.py, update-.py) at the repo root are development artifacts that shouldn't be in the final changeset. Please remove them.
There are still references to the old .kilocode paths in files not touched by this PR (kilo-ignore.ts, gitignore-entries.ts). Per the project's feature-change-guidelines.md, those should be updated and pnpm dev gitignore should be run.
The kilo-hooks.ts and kilo-subagent.ts files are near-identical copies of their OpenCode counterparts. Since Kilo is built on OpenCode and shares the same plugin architecture, extracting shared logic into a common module would reduce maintenance burden. Not necessarily a blocker, but worth considering.
The duplicated constants in hooks.ts (KILO_HOOK_EVENTS, CANONICAL_TO_KILO_EVENT_NAMES) are byte-for-byte identical to the OpenCode ones. If they're expected to stay in sync, referencing the same constant would be simpler. If they might diverge, a comment explaining that would help.
fix-hooks-processor.py
Outdated
| @@ -0,0 +1,10 @@ | |||
| with open('src/features/hooks/hooks-processor.ts', 'r') as f: | |||
There was a problem hiding this comment.
This file (and the other Python scripts at the root: patch-*.py, update-*.py) look like development scaffolding used to generate the Kilo code via find-and-replace. They should be removed before merging — they expose the generation strategy and could destructively overwrite source files if accidentally executed.
| } from "./tool-command.js"; | ||
|
|
||
| export type KiloCommandParams = AiFileParams; | ||
| export const KiloCommandFrontmatterSchema = z.looseObject({ |
There was a problem hiding this comment.
Mixing z.optional(...) and bare optional(...) here. The rest of the codebase consistently uses z.optional(). Consider normalizing to z.optional() for all fields.
src/types/hooks.ts
Outdated
| "stop", | ||
| "afterFileEdit", | ||
| "afterShellExecution", | ||
| "permissionRequest", |
There was a problem hiding this comment.
KILO_HOOK_EVENTS and CANONICAL_TO_KILO_EVENT_NAMES are byte-for-byte identical to the OpenCode equivalents. If they are expected to stay in sync, you could just re-export the OpenCode constants under the Kilo name. If they might diverge later, a brief comment noting that would clarify intent.
| @@ -0,0 +1,241 @@ | |||
| import { join } from "node:path"; | |||
There was a problem hiding this comment.
This file is nearly identical to opencode-hooks.ts — the core logic (escapeForTemplateLiteral, validateAndSanitizeMatcher, groupByEvent, generatePluginCode) is duplicated verbatim. Since Kilo is built on the same OpenCode foundation, extracting these shared utilities into a common module and injecting tool-specific config (event names, paths) would make both easier to maintain.
| @@ -0,0 +1,191 @@ | |||
| import { basename, join } from "node:path"; | |||
There was a problem hiding this comment.
Same pattern as kilo-hooks.ts — this is a near-line-for-line copy of opencode-subagent.ts. The schema, constructor, toRulesyncSubagent, fromRulesyncSubagent, validate, fromFile, and forDeletion methods are structurally identical, differing only in class/type names, directory paths, and the rulesync frontmatter key. A parameterized base or factory approach would eliminate this duplication.
| return { success: true, error: null }; | ||
| } | ||
| if (!this.frontmatter) { | ||
| return { success: true, error: null }; |
There was a problem hiding this comment.
This if (!this.frontmatter) guard always passes since frontmatter is a required constructor parameter and assigned as readonly. The guard silently returns success for a falsy value, which could mask issues if the type system is bypassed. Consider removing this dead code path.
sirmacik
force-pushed
the
fix/kilo-opencode-rebase-1411-private
branch
from
c4f25c8 to
965e1cc
Compare
|
All review feedback is addressed, 4,746 tests are passing, and the branch is ready for merge. Resolutions Applied:
Action Required: Please review the updated diff and merge. |
| { target: "kilo", feature: "rules", entry: "**/.kilo/rules/" }, | ||
| { target: "kilo", feature: "skills", entry: "**/.kilo/skills/" }, | ||
| { target: "kilo", feature: "commands", entry: "**/.kilo/workflows/" }, | ||
| { target: "kilo", feature: "mcp", entry: "**/.kilo/mcp.json" }, | ||
| { target: "kilo", feature: "ignore", entry: "**/.kiloignore" }, |
There was a problem hiding this comment.
The Kilo gitignore entries for commands and MCP don’t match the updated Kilo implementations in this PR. KiloCommand.getSettablePaths() now uses .kilo/commands (and .config/kilo/commands for global), but this registry still ignores **/.kilo/workflows/. Also KiloMcp now reads/writes kilo.json/kilo.jsonc (project root) and .config/kilo/kilo.json (global), but the registry still ignores **/.kilo/mcp.json. Please update these entries to reflect the actual generated paths (and adjust tests accordingly).
| expect(content).toContain("**/.kilocode/workflows/"); | ||
| expect(content).toContain("**/.kilo/skills/"); | ||
| expect(content).toContain("**/.kilo/rules/"); | ||
| expect(content).toContain("**/.kilo/workflows/"); |
There was a problem hiding this comment.
This test asserts **/.kilo/workflows/ is included, but KiloCommand.getSettablePaths() now points to .kilo/commands in this PR. Update the expected pattern(s) to match the new Kilo commands directory (and consider also covering the MCP filename change if .gitignore/registry are updated).
| expect(content).toContain("**/.kilo/workflows/"); | |
| expect(content).toContain("**/.kilo/commands/"); |
| vi.mocked(KiloCommand).getSettablePaths = vi | ||
| .fn() | ||
| .mockReturnValue({ relativeDirPath: join(".kilocode", "workflows") }); | ||
| .mockReturnValue({ relativeDirPath: join(".kilo", "workflows") }); |
There was a problem hiding this comment.
The mocked Kilo command settable path is still .kilo/workflows, but KiloCommand.getSettablePaths() was changed to .kilo/commands in this PR. This mismatch can make the processor test validate the wrong output paths—please update the mock to mirror the new implementation.
| .mockReturnValue({ relativeDirPath: join(".kilo", "workflows") }); | |
| .mockReturnValue({ relativeDirPath: join(".kilo", "commands") }); |
| static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { | ||
| if (global) { | ||
| return { | ||
| relativeDirPath: join(".config", "kilo"), | ||
| relativeFilePath: "kilo.json", | ||
| }; | ||
| } | ||
| return { | ||
| relativeDirPath: ".kilocode", | ||
| relativeFilePath: "mcp.json", | ||
| relativeDirPath: ".", | ||
| relativeFilePath: "kilo.json", | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
KiloMcp.getSettablePaths({ global: true }) and fromFile/fromRulesyncMcp support a global .config/kilo/* location, but McpProcessor currently marks the kilo MCP target as supportsGlobal: false (see src/features/mcp/mcp-processor.ts:213-222) and will omit it from global target selection. Either update the processor metadata to enable global MCP for Kilo or remove the global branch here so behavior is consistent.
| const json = JSON.parse(this.fileContent || "{}"); | ||
| const result = KiloConfigSchema.safeParse(json); | ||
| if (!result.success) { | ||
| return { success: false, error: result.error }; | ||
| } | ||
| return { success: true, error: null }; |
There was a problem hiding this comment.
validate() uses JSON.parse(this.fileContent), which will throw (not return a ValidationResult) if fileContent contains JSONC (comments/trailing commas). Since this class already uses jsonc-parser elsewhere, validate() should parse with parseJsonc (and catch parse errors) so validation failures are reported as { success: false, error } rather than throwing.
| const json = JSON.parse(this.fileContent || "{}"); | |
| const result = KiloConfigSchema.safeParse(json); | |
| if (!result.success) { | |
| return { success: false, error: result.error }; | |
| } | |
| return { success: true, error: null }; | |
| try { | |
| const json = parseJsonc(this.fileContent || "{}"); | |
| const result = KiloConfigSchema.safeParse(json); | |
| if (!result.success) { | |
| return { success: false, error: result.error }; | |
| } | |
| return { success: true, error: null }; | |
| } catch (error) { | |
| return { | |
| success: false, | |
| error: error instanceof Error ? error : new Error(String(error)), | |
| }; | |
| } |
| // Kilo Code | ||
| { target: "kilo", feature: "rules", entry: "**/.kilocode/rules/" }, | ||
| { target: "kilo", feature: "skills", entry: "**/.kilocode/skills/" }, | ||
| { target: "kilo", feature: "commands", entry: "**/.kilocode/workflows/" }, | ||
| { target: "kilo", feature: "mcp", entry: "**/.kilocode/mcp.json" }, | ||
| { target: "kilo", feature: "ignore", entry: "**/.kilocodeignore" }, | ||
| { target: "kilo", feature: "rules", entry: "**/.kilo/rules/" }, | ||
| { target: "kilo", feature: "skills", entry: "**/.kilo/skills/" }, | ||
| { target: "kilo", feature: "commands", entry: "**/.kilo/workflows/" }, | ||
| { target: "kilo", feature: "mcp", entry: "**/.kilo/mcp.json" }, | ||
| { target: "kilo", feature: "ignore", entry: "**/.kiloignore" }, |
There was a problem hiding this comment.
This PR adds Kilo hooks generation to .kilo/plugins/rulesync-hooks.js (and .config/kilo/plugins for global), but there is no corresponding gitignore entry for the generated Kilo hooks plugin. Consider adding an entry like **/.kilo/plugins/ (or the specific file) so generated hook plugins aren’t accidentally committed, consistent with other hook targets (e.g. **/.github/hooks/, **/.deepagents/hooks.json).
|
@sirmacik Thank you! |
Fixes #1411.
Updates the
kilotarget processors to match its new internal architecture (rebased onto OpenCode) while preserving the.kiloand.config/kilouser directory structures.Note for users: Kilo has a built-in migration feature to update legacy configurations transparently, so these path updates seamlessly align with the latest Kilo releases.
(Note: This PR replaces #1412 to correct author attribution)