feat: add support for Kilo Code#25
Conversation
- add import functionality - add appropriate testing - sort lists alphabetically to improve visual scanning
📝 WalkthroughWalkthroughAdds Kilocode import/export support across CLI, importers, exporters, types, public API, README, and tests; introduces memory-bank handling, gitignore integration, and extends importAll/exportAll orchestration and return shapes. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Importer as Importer (importKilocode)
participant Exporter as Exporter (exportToKilocode)
participant FS as Filesystem
CLI->>Importer: importAll(repoPath)
Importer->>FS: read .kilocode/rules/**/*.md
FS-->>Importer: file contents + frontmatter
Importer->>Importer: parse frontmatter, compute IDs, detect private, collect warnings
Importer-->>CLI: results + warnings
CLI->>Exporter: exportAll(rules, repoPath, options)
Exporter->>FS: write .kilocode/rules/{id}.md (frontmatter + body)
Exporter-->>CLI: exportedPaths
CLI->>FS: update .gitignore with exportedPaths
FS-->>CLI: .gitignore updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli.ts (1)
236-252:⚠️ Potential issue | 🟠 Major
.kilocode/rules/missing fromexportedPathsin the'all'case.
exportAll()(line 238) callsexportToKilocode, but theexportedPathslist here doesn't include'.kilocode/rules/'. This means the gitignore-update prompt (line 345) won't account for kilocode output when the user selects "all formats."Other recently added formats (
.roo/rules/,.junie/guidelines.md,.amazonq/rules/) are also missing, but.kilocode/rules/is directly introduced by this PR.🐛 Proposed fix
exportedPaths.push( '.github/copilot-instructions.md', '.cursor/rules/', '.clinerules', '.windsurfrules', '.rules', 'AGENTS.md', 'CONVENTIONS.md', 'CLAUDE.md', 'GEMINI.md', - 'best_practices.md' + 'best_practices.md', + '.amazonq/rules/', + '.roo/rules/', + '.kilocode/rules/', + '.junie/guidelines.md' )README.md (1)
246-263:⚠️ Potential issue | 🟡 MinorThe
updateGitignorefunction (lines 547–564) is missing patterns documented in the README but appears to be unused—consider removing it or clarifying its purpose.The private-patterns list in
updateGitignoreis missing entries for.junie/guidelines.local.md,.kilocode/rules/*.local.md, and.roo/rules/*.local.mdthat are documented in the README (lines 246–263). However, this function has no call sites in the codebase—the active gitignore update logic is inupdateGitignoreWithPaths(line 520, called at line 359). IfupdateGitignoreis dead code, it should be removed. If it's retained for future use, it should be updated to match the documented patterns.
🧹 Nitpick comments (5)
src/importers.ts (1)
913-980: Consider extracting a shared helper for directory-based importers.
importKilocodeis effectively identical toimportRoo(lines 817–884)—and very similar toimportAgentandimportCursor—differing only in theformatstring. The recursive file-walk, sorting, frontmatter parsing, ID derivation, and private-rule detection logic is duplicated verbatim each time a new directory-based format is added.A shared helper like
importDirectoryRules(rulesDir, format)would reduce each new importer to a one-liner and eliminate a growing maintenance burden.♻️ Sketch of a shared helper
// Shared helper (conceptual) function importDirectoryRules(rulesDir: string, format: string): ImportResult { const rules: RuleBlock[] = [] function findMdFiles(dir: string, relativePath = ''): void { const entries = readdirSync(dir, { withFileTypes: true }) entries.sort((a: Dirent, b: Dirent) => { if (a.isDirectory() && !b.isDirectory()) return -1 if (!a.isDirectory() && b.isDirectory()) return 1 return a.name.localeCompare(b.name) }) for (const entry of entries) { const fullPath = join(dir, entry.name) const relPath = relativePath ? join(relativePath, entry.name) : entry.name if (entry.isDirectory()) { findMdFiles(fullPath, relPath) } else if (entry.isFile() && entry.name.endsWith('.md')) { const content = readFileSync(fullPath, 'utf-8') const { data, content: body } = matter(content, grayMatterOptions) let segments = relPath.replace(/\.md$/, '').replace(/\\/g, '/').split('/') .map((s: string) => s.replace(/^\d{2,}-/, '').replace(/\.local$/, '')) if (segments[0] === 'private') segments = segments.slice(1) const defaultId = segments.join('/') const isPrivateFile = isPrivateRule(fullPath) const metadata: any = { id: data.id || defaultId, ...data } if (metadata.alwaysApply === undefined) metadata.alwaysApply = false if (data.private === true || (data.private === undefined && isPrivateFile)) metadata.private = true rules.push({ metadata, content: body.trim() }) } } } findMdFiles(rulesDir) return { format: format as any, filePath: rulesDir, rules } } // Then each importer becomes: export function importKilocode(rulesDir: string): ImportResult { return importDirectoryRules(rulesDir, 'kilocode') } export function importRoo(rulesDir: string): ImportResult { return importDirectoryRules(rulesDir, 'roo') }src/exporters.ts (1)
520-573: Same duplication note as the importer side.
exportToKilocodeis identical toexportToRooexcept for the target directory (.kilocode/rulesvs.roo/rules). A shared helper parameterized by the subdirectory path would consolidate both. Same recommendation asimportKilocode.test/export-functionality.test.ts (1)
137-138: Customdirnamehelper is fragile on Windows — minor concern.Line 143 calls
dirname(fullPath)wherefullPathis built withpath.join(). On Windows,joinproduces backslash-separated paths, but the customdirnameat lines 172-174 splits only on'/'. This would causedirnameto return the full path unchanged on Windows, making theexistsSync(dirname(fullPath))fallback ineffective.Consider importing
dirnamefrompath(renaming the local helper or the import) for cross-platform correctness:-import { join } from 'path' +import { join, dirname as pathDirname } from 'path'Then use
pathDirnamein line 143 instead of the custom helper, or keep the custom one but usepath.sepor a cross-platform split.That said, this is pre-existing code and tests typically run on Linux/Mac CI, so impact is low.
Also applies to: 141-144
test/integration/kilocode.test.ts (2)
91-141: Naming overlap betweenprivate/directory andprivate: truemetadata could confuse future readers.In the test data, the rule at line 102 has
id: 'private/exported'(makingprivatea directory name) andmetadata.private: true. While functionally correct, this dual use of "private" may make the test harder to reason about in the future.Consider renaming the ID to something like
'internal/exported'to separate the directory-nesting concern from the visibility concern. Purely optional.
184-204: Smoke test is fine but the name oversells "CLI" coverage.The test is titled "CLI export to kilocode format creates correct structure" but it directly calls
exportToKilocode, not any CLI entrypoint. The comment on line 185 acknowledges this, which is good. Consider renaming to something like'exports to kilocode format with correct directory structure'to avoid confusion about what's actually being tested.
- Add JSDoc comments to 39 functions across cli.ts, importers.ts, and exporters.ts - Follow existing codebase convention for simple one-line descriptions - Addresses CodeRabbit docstring coverage warning
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/exporters.ts (1)
664-667:⚠️ Potential issue | 🟡 MinorPre-existing:
exportToCodexandexportToOpenCodeboth write toAGENTS.md— second call silently overwrites the first.Lines 664 and 667 both target
join(repoPath, 'AGENTS.md'). The OpenCode export will always overwrite the Codex export. This is not introduced by this PR, but it's worth flagging since it affects theexportAllpath that now includes Kilocode.src/cli.ts (1)
547-571:⚠️ Potential issue | 🟡 Minor
updateGitignoreWithPathsappends all patterns (including already-existing ones) when any new pattern is found.When
newPatterns.length > 0(line 563), the function appends the fullpatternsstring (built from allpaths, line 550–554) rather than just the new patterns. This means previously-added patterns will be duplicated in.gitignoreon subsequent runs if at least one new format path was added since the last run.🐛 Proposed fix — only append genuinely new patterns
if (newPatterns.length > 0) { - appendFileSync(gitignorePath, patterns) + const newSection = [ + '', + '# Added by dotagent: ignore exported AI rule files', + ...newPatterns.map(p => p.endsWith('/') ? p + '**' : p), + '' + ].join('\n') + appendFileSync(gitignorePath, newSection) return true }
🤖 Fix all issues with AI agents
In `@test/gitignore-all-formats.test.ts`:
- Around line 133-144: The custom failure messages are currently dead due to the
comma operator after expect(...).toContain(...); update the assertions
referencing EXPORT_FORMAT_PATHS and gitignoreContent so the message is passed as
the second argument to expect (e.g. expect(gitignoreContent, `Gitignore should
contain pattern for ${formatPath}...`).toContain(pattern)) and remove the stray
comma-separated template literals; do this for both the directory-branch
(pattern) and non-directory branch (formatPath) assertions so messages are shown
on test failures.
- Around line 75-93: The test helper runDotAgentExport currently shells out to a
built artifact at dist/cli.js (cliPath) which is gitignored and requires a prior
build; change the helper so tests run against the source rather than a built
file: update runDotAgentExport to invoke the TypeScript entry (e.g., src/cli.ts)
via a runtime TypeScript loader (node -r ts-node/register or npx ts-node) or
import/require the CLI module directly in tests and call its exported entry
function; ensure runDotAgentExport references the source path instead of
dist/cli.js (symbol: cliPath) or calls the CLI function exported by the module
so local pnpm test does not require a manual pnpm build.
🧹 Nitpick comments (5)
src/exporters.ts (1)
565-621:exportToKilocodeis a near-exact copy ofexportToRoo— consider extracting a shared helper.The entire body of
exportToKilocode(lines 568–621) is identical toexportToRoo(lines 510–563), differing only in the output directory name (.kilocode/rulesvs.roo/rules). The same frontmatter construction pattern is also repeated inexportToAgentandexportToCursor.A shared helper like
exportToMultiFileWithFrontmatter(rules, outputDir, subPath, options)would eliminate ~200+ lines of duplication across these four exporters and make adding future formats trivial.♻️ Sketch of shared helper
+function exportMultiFileMd( + rules: RuleBlock[], + outputDir: string, + subPath: string, + options?: ExportOptions +): void { + const rulesDir = join(outputDir, ...subPath.split('/')) + mkdirSync(rulesDir, { recursive: true }) + + const filteredRules = rules.filter(rule => !rule.metadata.private || options?.includePrivate) + + for (const rule of filteredRules) { + let filePath: string + if (rule.metadata.id && rule.metadata.id.includes('/')) { + const parts = rule.metadata.id.split('/') + const fileName = parts.pop() + '.md' + const subDir = join(rulesDir, ...parts) + mkdirSync(subDir, { recursive: true }) + filePath = join(subDir, fileName) + } else { + filePath = join(rulesDir, `${rule.metadata.id || 'rule'}.md`) + } + + const frontMatter = buildDeterministicFrontMatter(rule.metadata) + const content = rule.content.startsWith('\n') ? rule.content : '\n' + rule.content + writeFileSync(filePath, matter.stringify(content, frontMatter, grayMatterOptions), 'utf-8') + } +} + export function exportToRoo(rules: RuleBlock[], outputDir: string, options?: ExportOptions): void { - // ... 50+ lines ... + exportMultiFileMd(rules, outputDir, '.roo/rules', options) } export function exportToKilocode(rules: RuleBlock[], outputDir: string, options?: ExportOptions): void { - // ... 50+ lines ... + exportMultiFileMd(rules, outputDir, '.kilocode/rules', options) }src/importers.ts (3)
1067-1098: Import function with file-write side effects is surprising — consider separating the memory-bank migration.
importKilocodewrites to.agents/common-tasks.md(lines 1088–1095) during what callers would expect to be a read-only import operation. This couples data reading with file system mutation, making the function harder to test and reason about. Callers ofimportKilocodevia the public API (re-exported fromsrc/index.ts) may not expect side effects.Consider returning the memory-bank info in the result (e.g., via
warningsor a dedicated field) and performing the file write in the CLI or orchestration layer instead.
1069-1076:skipMemoryBankWarningname is misleading — both paths emit a message.The flag doesn't actually "skip" a warning; it selects which message to emit (lines 1167–1173). A name like
memoryBankImportedortasksMigratedwould better convey the intent —truewhen the copy succeeded,falsewhen skipped due to existing file.
1056-1061: Unnecessaryas anycast —'junie'is a valid member of theformatunion.
'junie'was added toImportResult['format']insrc/types.ts(line 24), so this type assertion is no longer needed.♻️ Proposed fix
return { - format: 'junie' as any, + format: 'junie', filePath, rules, raw: content }test/export-functionality.test.ts (1)
148-168: Format mapping test is missing several formats:gemini,roo,junie,amazonq.The
formatMapcovers 11 of the 15 non-allformats. While this is a pre-existing gap, it's worth completing since the test's purpose is to validate the mapping. Adding the missing entries would catch future wiring mistakes.
… artifact The test helper now imports CLI functions directly from src/index.ts instead of shelling out to dist/cli.js. This allows tests to run without a prior build step. Changes: - Replace execSync with direct imports of export functions - Add helper functions for gitignore pattern filtering and updating - Maintain existing test coverage for export and gitignore functionality
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli.ts (1)
556-581:⚠️ Potential issue | 🟡 Minor
updateGitignoreWithPathsappends all patterns when any new one is found, potentially duplicating existing entries.Lines 570–575:
filterNewPatternscorrectly identifies which patterns are new, but the function always appends the fullpatternsstring (built from allpathson lines 559–564). If some paths already exist in.gitignore, they'll be added again. Consider buildingpatternsfromnewPatternsinstead.♻️ Proposed fix
if (existsSync(gitignorePath)) { const content = readFileSync(gitignorePath, 'utf-8') // Check if any of the patterns already exist const newPatterns = filterNewPatterns(content, paths) if (newPatterns.length > 0) { - appendFileSync(gitignorePath, patterns) + const newBlock = [ + '', + '# Added by dotagent: ignore exported AI rule files', + ...newPatterns.map(p => p.endsWith('/') ? p + '**' : p), + '' + ].join('\n') + appendFileSync(gitignorePath, newBlock) return true }
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 99-102: Remove or gate the ad-hoc debug console.log calls that
print color.dim(`[DEBUG] ...`) (references: repoPath, process.cwd(),
existsSync(join(..., '.agent'))) so they don't appear in normal CLI runs;
replace them by honoring a verbose flag (e.g., --verbose or a verbose boolean
parsed in the import command handler) and only call those console.log lines when
verbose is true, and apply the same change to the other debug lines using
color.dim at the other reported spots (the additional debug logs at lines
referencing the same pattern). Ensure the CLI option parsing sets a verbose
variable reachable in the import command scope and use it to gate the debug
prints instead of unconditional console.log.
In `@src/importers.ts`:
- Around line 1143-1153: The current warning logic using foundMemoryBank and
memoryBankImported produces a misleading message when the memory-bank directory
exists but tasks.md was never found; update the messages in the block that
builds warnings (the warnings array) to accurately reflect the state: when
foundMemoryBank && memoryBankImported keep the existing 'memory-bank/tasks.md
found - memory bank is available' message, but when foundMemoryBank &&
!memoryBankImported change the warning to something like 'memory-bank directory
exists but tasks.md not found or not imported' (or split into two clearer
messages if you add a separate flag for tasksMdFound); ensure you modify the
code that references foundMemoryBank and memoryBankImported so the text matches
the actual condition.
🧹 Nitpick comments (9)
src/importers.ts (3)
15-376: Massive repetition inimportAll— each format block is identical boilerplate.Every format follows the exact same try/catch + push + warnings pattern (about 30 near-identical blocks). A helper function would eliminate this duplication and make adding new formats a one-liner.
♻️ Suggested refactor
+ // Helper to run an importer and collect results/warnings/errors + function tryImport(path: string, importer: (p: string) => ImportResult): void { + if (!existsSync(path)) return + try { + const result = importer(path) + results.push(result) + if (result.warnings) { + warnings.push(...result.warnings) + } + } catch (e) { + errors.push({ file: path, error: String(e) }) + } + } // Then replace each block with: - const agentDir = join(repoPath, '.agent') - if (existsSync(agentDir)) { - try { - const result = importAgent(agentDir) - results.push(result) - if (result.warnings) { - warnings.push(...result.warnings) - } - } catch (e) { - errors.push({ file: agentDir, error: String(e) }) - } - } + tryImport(join(repoPath, '.agent'), importAgent)
1067-1161:importKilocodeis nearly identical toimportRoo(lines 965–1032) apart from memory-bank handling and the format string.The core logic — recursive MD discovery, frontmatter parsing, segment-based ID derivation, private detection — is copy-pasted across both (and several other importers). Consider extracting a shared helper that accepts format-specific options (format name, skip-directories, etc.) to reduce duplication.
1069-1077:memoryBankImportedis misleading — nothing is actually imported from the memory bank.The variable is set to
truewhentasks.mdexists, but the file content is never read or added to rules. The variable name suggests a side effect that doesn't occur. Consider renaming tomemoryBankTasksFoundor similar to match the actual semantics.src/cli.ts (1)
434-477:importKilocodeis both statically imported (line 6) and dynamically imported (line 435) — the dynamic import shadows the static one.This works but is inconsistent:
importRooat line 473 uses the static import (not in the dynamic destructuring), whileimportKilocodeat line 476 uses the dynamic one. Consider addingimportRooto the dynamic import or removingimportKilocodefrom it to keep the approach uniform.test/gitignore-all-formats.test.ts (3)
185-231:filterNewPatternsandupdateGitignoreWithPathsare duplicated fromsrc/cli.ts.These helpers are copy-pasted from the production code. If the CLI implementation changes, the test helpers could silently diverge. Consider exporting these utilities from a shared module (e.g.,
src/utils/gitignore.ts) and importing them in bothcli.tsand this test file.
90-107:exportedPathsinrunDotAgentExportduplicatesEXPORT_FORMAT_PATHS— use the constant instead.The
'all'branch hardcodes the same list thatEXPORT_FORMAT_PATHSalready defines. If a new format is added to one but not the other, the test could silently pass or fail for the wrong reason.♻️ Proposed fix
if (format === 'all') { exportAll(rules, tempDir, false) - exportedPaths.push( - '.amazonq/rules/', - '.clinerules', - '.cursor/rules/', - '.github/copilot-instructions.md', - '.junie/guidelines.md', - '.kilocode/rules/', - '.roo/rules/', - '.rules', - '.windsurfrules', - 'AGENTS.md', - 'best_practices.md', - 'CLAUDE.md', - 'CONVENTIONS.md', - 'GEMINI.md' - ) + exportedPaths.push(...EXPORT_FORMAT_PATHS) } else {
316-329: Test name is misleading — it doesn't test failure behavior.The test is named
"fails with clear message when exportedPaths is incomplete"but it only asserts that certain paths exist in theEXPORT_FORMAT_PATHSconstant. There's no test of actual failure behavior or error messages. Consider renaming to something like"EXPORT_FORMAT_PATHS includes all expected format paths".test/export-functionality.test.ts (1)
148-172: Format mapping test only validates function existence, not actual export behavior.This test checks
typeof config.exporter === 'function'andconfig.pathis truthy, but doesn't invoke any exporter. It serves as a documentation/registration check, which is fine, but consider noting that it doesn't replace integration tests for each format.test/integration/kilocode.test.ts (1)
303-327: Test setup creates.agents/directory that is irrelevant to assertions.Lines 313–316 create
.agents/common-tasks.md(note:.agentswith an 's', not.agent), but no assertion references it. The test only checkswarningsfromimportAll. This setup is dead code from a likely earlier design whereimportKilocodewould write to.agents/common-tasks.md. Consider removing the unused setup or updating the test name/comments to clarify intent.
| // Debug logging | ||
| console.log(color.dim(`[DEBUG] Resolved repoPath: ${repoPath}`)) | ||
| console.log(color.dim(`[DEBUG] Current working directory: ${process.cwd()}`)) | ||
| console.log(color.dim(`[DEBUG] .agent directory exists: ${existsSync(join(repoPath, '.agent'))}`)) |
There was a problem hiding this comment.
Remove debug logging from production CLI output.
These [DEBUG] statements (also at lines 145, 150–152) will clutter the output for every user running dotagent import. They should be removed or gated behind a --verbose flag.
- // Debug logging
- console.log(color.dim(`[DEBUG] Resolved repoPath: ${repoPath}`))
- console.log(color.dim(`[DEBUG] Current working directory: ${process.cwd()}`))
- console.log(color.dim(`[DEBUG] .agent directory exists: ${existsSync(join(repoPath, '.agent'))}`))Lines 145, 150–152 similarly:
- console.log(color.dim(`[DEBUG] Dry-run mode: .agent directory would NOT be created`))- console.log(color.dim(`[DEBUG] .agent directory created at: ${agentDir}`))
- console.log(color.dim(`[DEBUG] outputDir: ${outputDir}`))
- console.log(color.dim(`[DEBUG] .agent exists after export: ${existsSync(join(outputDir, '.agent'))}`))🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 99 - 102, Remove or gate the ad-hoc debug
console.log calls that print color.dim(`[DEBUG] ...`) (references: repoPath,
process.cwd(), existsSync(join(..., '.agent'))) so they don't appear in normal
CLI runs; replace them by honoring a verbose flag (e.g., --verbose or a verbose
boolean parsed in the import command handler) and only call those console.log
lines when verbose is true, and apply the same change to the other debug lines
using color.dim at the other reported spots (the additional debug logs at lines
referencing the same pattern). Ensure the CLI option parsing sets a verbose
variable reachable in the import command scope and use it to gate the debug
prints instead of unconditional console.log.
| // Build warnings array | ||
| const warnings: string[] = [] | ||
| if (foundMemoryBank) { | ||
| if (memoryBankImported) { | ||
| // tasks.md was found, indicates memory bank is available | ||
| warnings.push('memory-bank/tasks.md found - memory bank is available') | ||
| } else { | ||
| // Memory bank exists but wasn't processed | ||
| warnings.push('memory-bank/tasks.md found but not imported') | ||
| } | ||
| } |
There was a problem hiding this comment.
Misleading warning when memory-bank directory exists without tasks.md.
When the memory-bank directory exists but tasks.md is absent inside it, foundMemoryBank will be true (set at line 1097) while memoryBankImported stays false (only set at line 1076). The warning at line 1151 will say "memory-bank/tasks.md found but not imported", which is misleading because tasks.md was never found — only the directory was.
🐛 Proposed fix
if (foundMemoryBank) {
if (memoryBankImported) {
// tasks.md was found, indicates memory bank is available
warnings.push('memory-bank/tasks.md found - memory bank is available')
} else {
- // Memory bank exists but wasn't processed
- warnings.push('memory-bank/tasks.md found but not imported')
+ // Memory bank directory exists but tasks.md was not found
+ warnings.push('memory-bank directory found but tasks.md is missing')
}
}🤖 Prompt for AI Agents
In `@src/importers.ts` around lines 1143 - 1153, The current warning logic using
foundMemoryBank and memoryBankImported produces a misleading message when the
memory-bank directory exists but tasks.md was never found; update the messages
in the block that builds warnings (the warnings array) to accurately reflect the
state: when foundMemoryBank && memoryBankImported keep the existing
'memory-bank/tasks.md found - memory bank is available' message, but when
foundMemoryBank && !memoryBankImported change the warning to something like
'memory-bank directory exists but tasks.md not found or not imported' (or split
into two clearer messages if you add a separate flag for tasksMdFound); ensure
you modify the code that references foundMemoryBank and memoryBankImported so
the text matches the actual condition.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
Chores