Skip to content

docs(cli): clarify hidden .kilo install artifact behavior#6540

Merged
catrielmuller merged 4 commits into
Kilo-Org:mainfrom
sonwr:docs/clarify-kilo-install-artifact
Mar 3, 2026
Merged

docs(cli): clarify hidden .kilo install artifact behavior#6540
catrielmuller merged 4 commits into
Kilo-Org:mainfrom
sonwr:docs/clarify-kilo-install-artifact

Conversation

@sonwr

@sonwr sonwr commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR clarifies the hidden .kilo file that some users see after npm-based CLI installs, and adds a small regression test around install artifact behavior.

What changed

  • Added a short README note explaining:
    • what .kilo is,
    • why it may appear,
    • why size can vary by platform/npm mode,
    • and safe uninstall guidance.
  • Added packages/opencode/test/cli/install-artifact.test.ts to verify:
    • wrapper contract assumptions,
    • local npm install bin-link behavior for kilo and kilocode,
    • and optional .kilo artifact shape when present.

Validation

  • cd packages/opencode && bun test test/cli/install-artifact.test.ts ✅ (2 passed)

Fixes #6521

@vercel

vercel Bot commented Mar 3, 2026

Copy link
Copy Markdown

@sonwr is attempting to deploy a commit to the Kilo Code Team on Vercel.

A member of the Team first needs to authorize it.

})

test("links npm bin commands to the wrapper during local install", async () => {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "kilo-install-artifact-"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Missing temp directory cleanup

fs.mkdtemp creates a temporary directory that is never cleaned up after the test completes. Over repeated test runs (especially in CI), this will accumulate orphaned directories in the system temp folder.

Consider adding cleanup in a finally block or using afterAll/afterEach:

let tmp: string
afterEach(async () => {
  if (tmp) await fs.rm(tmp, { recursive: true, force: true })
})

@kilo-code-bot

kilo-code-bot Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No New Issues Found | Recommendation: Merge

Overview

This PR adds a README note explaining the hidden .kilo file that may appear after npm-based CLI installs, and a regression test (install-artifact.test.ts) verifying wrapper contract assumptions and npm bin-link behavior.

Both changes are clean and well-structured:

  • The README documentation is clear and helpful for users encountering the .kilo artifact.
  • The test file properly handles npm unavailability (skip with warning), uses temp directories with cleanup in a finally block, and treats the .kilo artifact as optional.
  • kilocode_change markers are correctly applied.

No security vulnerabilities, runtime errors, logic bugs, or breaking changes detected.

Files Reviewed (2 files)
  • README.md — Documentation addition explaining .kilo install artifact
  • packages/opencode/test/cli/install-artifact.test.ts — New test for CLI wrapper contract and npm install artifact behavior

@sonwr

sonwr commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review — great catch.\n\nI added temp directory cleanup in the test using a block with to avoid orphaned directories across repeated runs/CI.\n\nAlso re-ran the focused test:\n- \n- Result: 2 passed

@sonwr

sonwr commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Apologies — reposting cleanly.

Thanks for the careful review — great catch.

I added temp directory cleanup in the test using a try/finally block with:
fs.rm(tmp, { recursive: true, force: true })
so repeated runs/CI do not leave orphaned temp directories.

Re-ran focused test:

  • cd packages/opencode && bun test test/cli/install-artifact.test.ts
  • Result: 2 passed

Comment thread packages/opencode/test/cli/install-artifact.test.ts
@sonwr

sonwr commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Quick note: the Vercel – docs check appears to fail due to fork authorization requirements (Authorization required to deploy), not due to code/test failures in this PR.

I re-ran the focused test locally after the cleanup fix:

  • cd packages/opencode && bun test test/cli/install-artifact.test.ts
  • Result: 2 passed

@sonwr

sonwr commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the follow-up review — addressed.

I added an explicit npm availability guard in the test:

  • if npm is not in PATH, the test now exits early with a clear skip message.

This keeps CI behavior explicit in environments that only provide Bun.

Re-ran focused test:

  • cd packages/opencode && bun test test/cli/install-artifact.test.ts
  • Result: 2 passed

@catrielmuller
catrielmuller merged commit 8e0e368 into Kilo-Org:main Mar 3, 2026
8 of 9 checks passed
slamj1 pushed a commit to slamj1/kilocode that referenced this pull request May 16, 2026
jliounis pushed a commit to jliounis/kilocode that referenced this pull request May 18, 2026
jliounis pushed a commit to jliounis/kilocode that referenced this pull request May 18, 2026
…-artifact

docs(cli): clarify hidden .kilo install artifact behavior
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Large hidden .kilo file created during npm install

2 participants