-
-
Notifications
You must be signed in to change notification settings - Fork 142
feat(kilo): explicitly declare supported Kilo agent frontmatter fields #1655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fc9c402
9a21256
ba3e0a1
02e9c27
6585031
219a8fc
16a8ff7
f7af83e
569aa1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { join } from "node:path"; | |
|
|
||
| import { z } from "zod/mini"; | ||
|
|
||
| import { ValidationResult } from "../../types/ai-file.js"; | ||
| import { ToolTarget } from "../../types/tool-targets.js"; | ||
| import { formatError } from "../../utils/error.js"; | ||
| import { readFileContent } from "../../utils/file.js"; | ||
|
|
@@ -18,19 +19,65 @@ import { | |
|
|
||
| export const KiloSubagentFrontmatterSchema = z.looseObject({ | ||
| description: z.optional(z.string()), | ||
| // Kilo's documented default for user-defined agents is "all": | ||
| // available both as a top-level pick and as a subagent. | ||
| mode: z._default(z.string(), "all"), | ||
| name: z.optional(z.string()), | ||
| displayName: z.optional(z.string()), | ||
| deprecated: z.optional(z.boolean()), | ||
| native: z.optional(z.boolean()), | ||
| hidden: z.optional(z.boolean()), | ||
| top_p: z.optional(z.number()), | ||
| temperature: z.optional(z.number()), | ||
| color: z.optional(z.string()), | ||
| permission: z.optional(z.string()), | ||
| model: z.optional(z.string()), | ||
| variant: z.optional(z.string()), | ||
| prompt: z.optional(z.string()), | ||
| options: z.optional(z.looseObject({})), | ||
| steps: z.optional(z.array(z.looseObject({}))), | ||
| disable: z.optional(z.boolean()), | ||
| }); | ||
| export type KiloSubagentFrontmatter = z.infer<typeof KiloSubagentFrontmatterSchema>; | ||
| export type KiloSubagentParams = OpenCodeStyleSubagentParams; | ||
| export type KiloSubagentParams = Omit<OpenCodeStyleSubagentParams, "frontmatter"> & { | ||
| frontmatter: KiloSubagentFrontmatter; | ||
| }; | ||
|
|
||
| export class KiloSubagent extends OpenCodeStyleSubagent { | ||
| declare protected readonly frontmatter: KiloSubagentFrontmatter; | ||
|
|
||
| constructor(params: KiloSubagentParams) { | ||
| super(params); | ||
| if (params.validate !== false) { | ||
| const result = this.validate(); | ||
| if (!result.success) { | ||
| throw result.error; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| protected getToolTarget(): Extract<ToolTarget, "opencode" | "kilo"> { | ||
| return "kilo"; | ||
| } | ||
|
|
||
| getFrontmatter(): KiloSubagentFrontmatter { | ||
| return this.frontmatter; | ||
| } | ||
|
|
||
| validate(): ValidationResult { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One subtlety worth flagging: this override is not called from the parent constructor. |
||
| const result = KiloSubagentFrontmatterSchema.safeParse(this.frontmatter); | ||
| if (result.success) { | ||
| // @ts-expect-error - readonly | ||
| this.frontmatter = result.data; | ||
| return { success: true, error: null }; | ||
| } | ||
|
|
||
| return { | ||
| success: false, | ||
| error: new Error( | ||
| `Invalid frontmatter in ${join(this.relativeDirPath, this.relativeFilePath)}: ${formatError(result.error)}`, | ||
| ), | ||
| }; | ||
| } | ||
|
|
||
| static getSettablePaths({ | ||
| global = false, | ||
| }: { | ||
|
|
@@ -50,12 +97,19 @@ export class KiloSubagent extends OpenCodeStyleSubagent { | |
| const rulesyncFrontmatter = rulesyncSubagent.getFrontmatter(); | ||
| const kiloSection = rulesyncFrontmatter.kilo ?? {}; | ||
|
|
||
| const kiloFrontmatter: KiloSubagentFrontmatter = KiloSubagentFrontmatterSchema.parse({ | ||
| const parseResult = KiloSubagentFrontmatterSchema.safeParse({ | ||
| ...kiloSection, | ||
| description: rulesyncFrontmatter.description, | ||
| ...(rulesyncFrontmatter.name && { name: rulesyncFrontmatter.name }), | ||
| }); | ||
|
|
||
| if (!parseResult.success) { | ||
| throw new Error( | ||
| `Invalid frontmatter in ${rulesyncSubagent.getRelativeFilePath()}: ${formatError(parseResult.error)}`, | ||
| ); | ||
| } | ||
| const kiloFrontmatter: KiloSubagentFrontmatter = parseResult.data; | ||
|
|
||
| const body = rulesyncSubagent.getBody(); | ||
| const fileContent = stringifyFrontmatter(body, kiloFrontmatter); | ||
| const paths = this.getSettablePaths({ global }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All three new tests are happy-path. The headline behavior of the PR — that bad types now raise Zod errors — isn't covered, so a regression that, say, drops the
validate()override or loosensz.number()back toz.unknown()would pass CI. A single negative test (e.g.KiloSubagentFrontmatterSchema.safeParse({ mode: "subagent", temperature: "hot" })assertingresult.success === false, and/orKiloSubagent.fromFilerejecting a file with an invalid type) would lock in the strict behavior this PR is introducing.