Repository navigation
Conversation
| console.error("Failed to run lockfile check:", err) | ||
| process.exit(1) | ||
| } finally { | ||
| // Always restore the committed lockfile so the working tree stays clean. |
There was a problem hiding this comment.
WARNING: process.exit(1) skips the finally block
process.exit() terminates the process synchronously without unwinding the stack, so Node/Bun do not run finally blocks on exit. On the drift path (line 39) and the error path (line 43), the finally that restores the committed lockfile therefore never runs, leaving a regenerated bun.lock on disk. That contradicts the "always restore the committed lockfile" guarantee in the comment above and the PR's verification note that the working tree stays clean. Consider recording a failure flag and letting the finally run before calling process.exit.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "@kilocode/kilo-jetbrains": patch | ||
| --- | ||
|
|
||
| Regenerate bun.lock so the workspace entry for @kilocode/kilo-jetbrains matches package.json, and add a CI guard that fails when workspace package versions drift from bun.lock. No newline at end of file |
There was a problem hiding this comment.
SUGGESTION: Changeset reads as implementation notes
Changeset descriptions appear directly in release notes for end users, but this one describes the mechanics ("Regenerate bun.lock ... add a CI guard ..."). Consider a user-facing phrasing focused on the effect, for example: "Keep the JetBrains plugin building against the correct CLI version by keeping the lockfile workspace version in sync." If this change is purely internal tooling with no user-visible behavior, a changeset may not be needed at all.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The previous findings are resolved on Files Reviewed (5 files)
Previous Review Summaries (7 snapshots, latest commit 18bb6c6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 18bb6c6)Status: 2 Issues Found | Recommendation: Address before merge Overview
The earlier blocking issues are resolved: Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 21d755b)Status: 1 Issue Found | Recommendation: Address before merge Overview
The two earlier findings in Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 12afaea)Status: 2 Issues Found | Recommendation: Address before merge Overview
No blocking issues remain: the regenerated Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 2bddf4a)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (4 files)
The previous two findings ( Previous review (commit 62efc79)Status: No Issues Found | Recommendation: Merge Both findings from the previous review are resolved at HEAD, and the new Files Reviewed (5 files)
Previous review (commit 50eef33)Status: No Issues Found | Recommendation: Merge Both findings from the previous review were addressed in commit
Files Reviewed (4 files)
Previous review (commit 64f218a)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Reviewed by deepseek-flash-latest · Input: 61.1K · Output: 11.4K · Cached: 540.3K Review guidance: REVIEW.md from base branch |
|
Counter-validation found one integration fix before merge: the new setup-bun guard will reject the automated JetBrains CLI pin-bump PRs. Please update the pin-bump automation to regenerate and include Reproduced with the actual PR manifests and CI-pinned Bun 1.3.14: a manifest-only pin bump passes the frozen install, then fails the new guard. Clean-workspace, drift-detection, and error-path restoration checks otherwise passed; no other blockers found. |
720ff84 to
62efc79
Compare
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
62efc79 to
2bddf4a
Compare
| # from the workspace, so version drift cannot land silently. # kilocode_change | ||
| - name: Verify lockfile matches workspace | ||
| shell: bash | ||
| run: bun script/check-bun-lock-workspace.ts |
There was a problem hiding this comment.
CRITICAL: This guard fails on the PR as it stands
packages/kilo-jetbrains/package.json is at 7.8.9, but the committed bun.lock workspace entry still records 7.8.8 (bun.lock:365). This PR's diff does not include a regenerated bun.lock, despite the title, so the workspace version drift from #14616 is still present on the branch.
Because this step runs bun install --lockfile-only and fails on any byte difference, CI goes red immediately on this guard. Regenerate and commit bun.lock in this PR (bun install --lockfile-only) so the guard has something consistent to verify.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| async function pushLock(version: string, previous: string, branch: string) { | ||
| const entry = (await $`gh api ${`repos/${repo}/contents/bun.lock?ref=${branch}`} --jq .content`.text()).trim() | ||
| const text = Buffer.from(entry, "base64").toString("utf8") | ||
| const anchor = `"packages/kilo-jetbrains": {\n "name": "@kilocode/kilo-jetbrains",\n "version": "${previous}",` |
There was a problem hiding this comment.
WARNING: Anchor assumes the lockfile still holds the previous version
The anchor interpolates previous, which is read from origin/main's package.json, as the value expected in bun.lock. If origin/main is already drifted (it currently is: package.json 7.8.9 vs bun.lock 7.8.8), the anchor does not match, patched === text, and this function silently skips with a warning — leaving the automated pin-bump PR to be rejected by the new guard. Anchoring on the workspace path and name and replacing whatever version follows would be robust to an already-stale lockfile.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // by regenerating locally), so patch that line directly through the API | ||
| // instead of cloning the repo and re-running install. | ||
| async function pushLock(version: string, previous: string, branch: string) { | ||
| const entry = (await $`gh api ${`repos/${repo}/contents/bun.lock?ref=${branch}`} --jq .content`.text()).trim() |
There was a problem hiding this comment.
SUGGESTION: Contents API omits content for files over 1 MB
gh api .../contents/bun.lock --jq .content returns an empty value once the file exceeds 1 MB, and bun.lock is already around 672 KB. When it crosses that size, this call yields an empty string, the anchor is not found, and the update silently skips — again leaving the guard to fail the pin PR. Fetching the blob via the Git Data API (or the raw media type) avoids the size cap.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
Review findings addressed in 21d755b:
|
| const path = `repos/${repo}/contents/bun.lock` | ||
| // The JSON `content` field is empty for files above 1 MB, which bun.lock is | ||
| // approaching. The raw media type returns the bytes regardless of size. | ||
| const text = await $`gh api ${`${path}?ref=${branch}`} -H "Accept: application/vnd.github.raw"`.text() |
There was a problem hiding this comment.
WARNING: Raw read can change the file's bytes, which the guard compares exactly
The lockfile now round-trips through gh api stdout and Bun's $.text(). Neither is a byte-faithful file read: gh api terminates its output with a newline, and Bun shell text output normalizes trailing newlines. If either drops or adds one, the base64 written back on line 147 differs from the branch blob by a trailing newline only, and script/check-bun-lock-workspace.ts compares the committed lockfile byte-for-byte against bun install --lockfile-only output — so the pin-bump PR once again trips the guard this code is meant to satisfy. Reading the blob to a temp file (gh api ... --output <path>) and base64-encoding that file keeps the bytes intact.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # fields, so a stale workspace entry in bun.lock (e.g. a JetBrains pin bump that | ||
| # forgot to re-run install) passes silently. Fail CI when the lockfile drifts | ||
| # from the workspace, so version drift cannot land silently. # kilocode_change | ||
| - name: Verify lockfile matches workspace |
There was a problem hiding this comment.
SUGGESTION: This guard ignores the install input
Every other dependency step above is gated with if: inputs.install != 'false', but this new step is not. Twelve workflows pass install: false (for example check-forbidden-strings.yml, community-label.yml, stale-bot-notify.yml, and test-jetbrains.yml), so they now all run bun install --lockfile-only on every event, including issue-triggered automations that deliberately skip dependency work. That adds a lockfile-resolution pass and a new failure mode to jobs that opted out of it. Consider adding if: inputs.install != 'false' like the neighbouring steps, or moving the check into a dedicated job.
|
Both findings addressed in 1a9892e:
Verified with |
Fixes #14616.
Summary
recorded a stale version for the workspace package (7.7.9 on disk vs 7.8.1 in ). bun install v1.3.14 (0d9b296a)
HUSKY=0 skip install
Checked 2052 installs across 2274 packages (no changes) [467.00ms] validates external deps but not workspace fields, so the drift was invisible to CI and every local bun install v1.3.14 (0d9b296a)
HUSKY=0 skip install
Checked 2052 installs across 2274 packages (no changes) [260.00ms] left a dirty tree.
Saved bun.lock (2274 packages) [312.00ms], diffs the result against the committed lockfile, and fails on any difference. The committed lockfile is always restored so the working tree stays clean.
Verification
Adversarial review
Usage: bun [...flags] [...args]
Commands:
run ./my-script.ts Execute a file with Bun
lint Run a package.json script
test Run unit tests with Bun
x eslint Execute a package binary (CLI), installing if needed (bunx)
repl Start a REPL session with Bun
exec Run a shell script directly with Bun
install Install dependencies for a package.json (bun i)
add @zarfjs/zarf Add a dependency to package.json (bun a)
remove babel-core Remove a dependency from package.json (bun rm)
update zod Update outdated dependencies
audit Check installed packages for vulnerabilities
outdated Display latest versions of outdated dependencies
link [] Register or link a local npm package
unlink Unregister a local npm package
publish Publish a package to the npm registry
patch Prepare a package for patching
pm Additional package management utilities
info tailwindcss Display package metadata from the registry
why elysia Explain why a package is installed
build ./a.ts ./b.jsx Bundle TypeScript & JavaScript into a single file
init Start an empty Bun project from a built-in template
create next-app Create a new project from a template (bun c)
upgrade Upgrade to latest version of Bun.
feedback ./file1 ./file2 Provide feedback to the Bun team.
--help Print help text for command.
Learn more about Bun: https://bun.com/docs
Join our Discord community: https://bun.com/discord on PATH.