bun feedback
#22710
bun feedback
#22710
Conversation
|
Updated 9:04 PM PT - Sep 16th, 2025
❌ @Jarred-Sumner, your commit 607a0c9 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 22710That installs a local version of the PR into your bun-22710 --bun |
WalkthroughIntroduces a new “feedback” evaluation workflow: adds CLI help text, auto-run path to execute an embedded eval script, build-time embedding and bundling of eval/feedback.ts, and supporting README. No public APIs change; behavior triggers only under specific auto-run conditions without workspaces. Changes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (5)
src/cli/feedback.ts (4)
148-159: Persist email to default ~/.bun when BUN_INSTALL is unset (symmetry with read).You read from
$BUN_INSTALL/feedbackor~/.bun/feedbackbut only persist whenBUN_INSTALLis set. This leads to non‑persistence for default installs.Apply this diff:
async function persistEmailToBunInstall(email: string): Promise<void> { - const installRoot = process.env.BUN_INSTALL; - if (!installRoot) return; + const installRoot = process.env.BUN_INSTALL ?? path.join(os.homedir(), ".bun"); const emailFile = path.join(installRoot, "feedback"); try { await fsp.mkdir(path.dirname(emailFile), { recursive: true }); await fsp.writeFile(emailFile, `${email.trim()}\n`, "utf8"); } catch (error) { console.warn(`Unable to persist email to ${emailFile}:`, (error as Error).message); } } - if (process.env.BUN_INSTALL && !storedEmail) { + if (!storedEmail) { await persistEmailToBunInstall(normalizedEmail); }Also applies to: 561-563
8-9: Prefer the global Web File API over node:buffer’s File.Bun provides
Fileglobally; importing fromnode:bufferis unnecessary and couples this to Node shims.Apply this diff:
-import { File } from "node:buffer"; +// Use the global Web File API: File(No code changes needed at usage sites;
new File([...], name)continues to work.)Also applies to: 597-598
409-427: Consider bounding stdin size to avoid unbounded memory use.
readFromStdin()buffers all input. Cap size (e.g., 10–20MB) and error out with a friendly message if exceeded.
21-25: Tiny cleanup: remove redundant||fallbacks.Template strings here are never falsy, so
|| "?"/"✔"/"✖"won’t be used.src/cli/run_command.zig (1)
1606-1616: Auto‑run path for “bun feedback” works; add a debug log for traceability.Condition gating is correct and non‑intrusive. Add a scoped log when the path triggers.
Apply this diff:
if (ctx.filters.len == 0 and !ctx.workspaces and CLI.Cli.cmd != null and CLI.Cli.cmd.? == .AutoCommand) { if (bun.strings.eqlComptime(target_name, "feedback")) { const trigger = bun.pathLiteral("/[eval]"); var entry_point_buf: [bun.MAX_PATH_BYTES + trigger.len]u8 = undefined; const cwd = try std.posix.getcwd(&entry_point_buf); @memcpy(entry_point_buf[cwd.len..][0..trigger.len], trigger); ctx.runtime_options.eval.script = @embedFile("./feedback.ts"); + log("AutoCommand: running embedded feedback.ts", .{}); try Run.boot(ctx, entry_point_buf[0 .. cwd.len + trigger.len], null); Global.exit(0); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/cli.zig(1 hunks)src/cli/feedback.ts(1 hunks)src/cli/run_command.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/cli.zigsrc/cli/run_command.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/cli.zigsrc/cli/run_command.zig
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
src/cli/feedback.ts
🧠 Learnings (1)
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Use `bun bd <...cmd>` to run commands with your debug build of Bun
Applied to files:
src/cli.zig
🧬 Code graph analysis (1)
src/cli/feedback.ts (2)
src/js/node/fs.ts (1)
closeSync(1108-1113)src/js/thirdparty/undici.js (1)
FormData(14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (4)
src/cli/feedback.ts (2)
173-189: Good interactive fallback loop.The prompt retries until a valid email is entered; nice UX.
301-406: Interactive multiline editor looks solid.Proper raw-mode handling, cleanup, and Ctrl+C exits with 130. Nicely done.
src/cli.zig (1)
190-191: Help entry added for “feedback” is clear and consistent.Text aligns with existing help formatting. No functional risk.
src/cli/run_command.zig (1)
1606-1616: No duplicate "feedback" auto‑run block found (src/cli/run_command.zig:1612). Found a single @embedfile("./feedback.ts") at line 1612.
| const endpoint = process.env.BUN_FEEDBACK_URL || "https://bun.com/api/v1/feedback"; | ||
|
|
||
| const form = new FormData(); | ||
| form.append("email", normalizedEmail); | ||
| form.append("message", messageBody); | ||
| for (const file of positionalContent.files) { | ||
| form.append("files[]", new File([file.content], file.filename)); | ||
| } | ||
| form.append("bun_version", Bun.version_with_sha); | ||
| if (projectId) { | ||
| form.append("project_id", projectId); | ||
| } | ||
|
|
||
| const response = await fetch(endpoint, { | ||
| method: "POST", | ||
| body: form, | ||
| headers: { | ||
| "User-Agent": `bun-feedback/${Bun.version_with_sha}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix: use a stable Bun version string (avoid Bun.version_with_sha).
Bun.version_with_sha isn’t guaranteed in JS; it may be undefined. Use Bun.version (+ short Bun.revision when available) and apply it consistently to both the form field and User-Agent.
Apply this diff:
const projectId = getOldestGitSha();
const endpoint = process.env.BUN_FEEDBACK_URL || "https://bun.com/api/v1/feedback";
+ const bunVersion =
+ (Bun as any).version_with_sha ??
+ (Bun.revision ? `${Bun.version}+${String(Bun.revision).slice(0, 7)}` : Bun.version);
+
const form = new FormData();
form.append("email", normalizedEmail);
form.append("message", messageBody);
for (const file of positionalContent.files) {
- form.append("files[]", new File([file.content], file.filename));
+ form.append("files[]", new File([file.content], file.filename));
}
- form.append("bun_version", Bun.version_with_sha);
+ form.append("bun_version", bunVersion);
if (projectId) {
form.append("project_id", projectId);
}
const response = await fetch(endpoint, {
method: "POST",
body: form,
headers: {
- "User-Agent": `bun-feedback/${Bun.version_with_sha}`,
+ "User-Agent": `bun-feedback/${bunVersion}`,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const endpoint = process.env.BUN_FEEDBACK_URL || "https://bun.com/api/v1/feedback"; | |
| const form = new FormData(); | |
| form.append("email", normalizedEmail); | |
| form.append("message", messageBody); | |
| for (const file of positionalContent.files) { | |
| form.append("files[]", new File([file.content], file.filename)); | |
| } | |
| form.append("bun_version", Bun.version_with_sha); | |
| if (projectId) { | |
| form.append("project_id", projectId); | |
| } | |
| const response = await fetch(endpoint, { | |
| method: "POST", | |
| body: form, | |
| headers: { | |
| "User-Agent": `bun-feedback/${Bun.version_with_sha}`, | |
| }, | |
| }); | |
| const endpoint = process.env.BUN_FEEDBACK_URL || "https://bun.com/api/v1/feedback"; | |
| const bunVersion = | |
| (Bun as any).version_with_sha ?? | |
| (Bun.revision ? `${Bun.version}+${String(Bun.revision).slice(0, 7)}` : Bun.version); | |
| const form = new FormData(); | |
| form.append("email", normalizedEmail); | |
| form.append("message", messageBody); | |
| for (const file of positionalContent.files) { | |
| form.append("files[]", new File([file.content], file.filename)); | |
| } | |
| form.append("bun_version", bunVersion); | |
| if (projectId) { | |
| form.append("project_id", projectId); | |
| } | |
| const response = await fetch(endpoint, { | |
| method: "POST", | |
| body: form, | |
| headers: { | |
| "User-Agent": `bun-feedback/${bunVersion}`, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In src/cli/feedback.ts around lines 591 to 610, replace usage of
Bun.version_with_sha with a stable version string: compute a stableVersion by
using Bun.version and, if Bun.revision exists, append a short form of it (e.g.,
first 7 chars) or otherwise omit; then use this stableVersion for both
form.append("bun_version", ...) and the User-Agent header value. Ensure you
still fall back to a sensible default like "unknown" if Bun.version is missing,
and apply the same stableVersion consistently to the form field and the
"User-Agent" header.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/cli/feedback.ts (1)
590-610: Use a stable Bun version string; avoid Bun.version_with_sha (can be undefined).Compute once and reuse for form + User‑Agent. This mirrors prior feedback.
Apply this diff:
const projectId = getOldestGitSha(); const endpoint = process.env.BUN_FEEDBACK_URL || "https://bun.com/api/v1/feedback"; + const bunVersion = + (Bun as any).version_with_sha ?? + (Bun.revision ? `${Bun.version}+${String(Bun.revision).slice(0, 7)}` : Bun.version ?? "unknown"); + const form = new FormData(); form.append("email", normalizedEmail); form.append("message", messageBody); @@ - form.append("bun_version", Bun.version_with_sha); + form.append("bun_version", bunVersion); @@ - const response = await fetch(endpoint, { + const response = await fetch(endpoint, { method: "POST", body: form, headers: { - "User-Agent": `bun-feedback/${Bun.version_with_sha}`, + "User-Agent": `bun-feedback/${bunVersion}`, }, });
🧹 Nitpick comments (6)
src/cli/feedback.ts (6)
604-621: Add a network timeout; avoid hanging CLI on slow/unreachable endpoints.Use AbortController with a sane default (e.g., 30s) and clear it.
Apply this diff:
- const response = await fetch(endpoint, { + const controller = new AbortController(); + const timeoutMs = Number(process.env.BUN_FEEDBACK_TIMEOUT_MS ?? 30_000); + const timer = setTimeout(() => controller.abort(), timeoutMs); + const response = await fetch(endpoint, { method: "POST", body: form, headers: { "User-Agent": `bun-feedback/${bunVersion}`, }, + signal: controller.signal, }); + clearTimeout(timer);
148-159: Persisted email is PII; write with restrictive perms.Set mode 0600 on Unix to avoid world‑readable files.
Apply this diff:
- await fsp.writeFile(emailFile, `${email.trim()}\n`, "utf8"); + await fsp.writeFile(emailFile, `${email.trim()}\n`, { encoding: "utf8", mode: 0o600 });
162-171: Harden git email lookup with a timeout.Prevents rare hangs if git is misconfigured.
Apply this diff:
- const result = spawnSync("git", ["config", "user.email"], { + const result = spawnSync("git", ["config", "user.email"], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], + timeout: 2000, });
495-507: Limit git rev-list work; add timeout.
--max-parents=0can scan a lot. Add-n 1and timeout.Apply this diff:
- const result = spawnSync("git", ["rev-list", "--max-parents=0", "HEAD"], { + const result = spawnSync("git", ["rev-list", "--max-parents=0", "-n", "1", "HEAD"], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], + timeout: 2000, });
22-25: Unreachable fallbacks insymbols(template strings are never falsy).Tiny cleanup; drop
||fallbacks.Apply this diff:
- question: `${colors.cyan}?${colors.reset}` || "?", - check: `${colors.green}✔${colors.reset}` || "✔", - cross: `${colors.red}✖${colors.reset}` || "✖", + question: `${colors.cyan}?${colors.reset}`, + check: `${colors.green}✔${colors.reset}`, + cross: `${colors.red}✖${colors.reset}`,
148-159: Optional: persist email in the same place you read from.You read from
${BUN_INSTALL ?? ~/.bun}/feedbackbut only write whenBUN_INSTALLis set. Consider writing to the same resolved path to make “remembered email” work by default.Also applies to: 133-146
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/cli/feedback.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
src/cli/feedback.ts
🧠 Learnings (1)
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: For Node.js compatibility bugs, validate expected behavior in Node.js first, then confirm failure on Bun canary
Applied to files:
src/cli/feedback.ts
🧬 Code graph analysis (1)
src/cli/feedback.ts (2)
src/js/node/fs.ts (1)
closeSync(1108-1113)src/js/thirdparty/undici.js (1)
FormData(14-14)
🔇 Additional comments (1)
src/cli/feedback.ts (1)
604-610: Move Bun version out of User-Agent; add a custom header fallbackSome proxies/hosts strip/forbid overriding User-Agent — src/cli/feedback.ts currently sets "User-Agent":
bun-feedback/${Bun.version_with_sha}(≈lines 604–610). Put Bun.version_with_sha into a custom header (e.g.X-Bun-Feedback-Version) or keep User-Agent as a fixed token (bun-feedback) and add the custom header. Verify target runtimes accept custom User-Agent if you choose to keep it.
| type PositionalContent = { | ||
| messageParts: string[]; | ||
| files: { filename: string; content: string }[]; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Treat attachments as binary; avoid UTF‑8 corruption.
Files are read with "utf8" and stored as string, which corrupts binary (images, zips). Read as bytes and append directly.
Apply this diff:
-type PositionalContent = {
- messageParts: string[];
- files: { filename: string; content: string }[];
-};
+type PositionalContent = {
+ messageParts: string[];
+ files: { filename: string; content: Uint8Array }[];
+};
@@
- const fileContents = await fsp.readFile(filePath, "utf8");
- files.push({ filename: path.basename(filePath), content: fileContents });
+ const fileContents = await fsp.readFile(filePath);
+ files.push({ filename: path.basename(filePath), content: fileContents });
@@
- for (const file of positionalContent.files) {
- form.append("files[]", new File([file.content], file.filename));
- }
+ for (const file of positionalContent.files) {
+ form.append("files[]", new File([file.content], file.filename));
+ }Also applies to: 475-483, 596-598
🤖 Prompt for AI Agents
In src/cli/feedback.ts around lines 429-433 (and also update occurrences at
475-483 and 596-598): the code currently types file contents as string and reads
files with "utf8", which corrupts binary attachments; change the file content
type from string to a binary type (Buffer or Uint8Array), read files using
fs.readFile without an encoding to get a Buffer, and pass/append those raw bytes
when building attachments; also update any serialization or upload logic to
handle Buffer (convert to base64 only if the transport requires JSON, or supply
the Buffer directly to FormData/multipart APIs).
|
Please don't do that. This is obviously bad in every possible way. There are already Github Issues/Discussions and Discord, why aren't you satisfied with them? Why another option? It will only confuse users. Also, you have already been informed in X that there will be problems with spam – why would you create new problems for yourself? As a result, you will only waste time and will not be able to simply remove this feature after the release, taking on the burden of supporting a such useless thing. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/js/eval/feedback.ts (6)
547-547: Remove unnecessary spread operator.The spread operator creates a new array unnecessarily here. Since
process.argv.slice(2)already returns a new array, spreading it serves no purpose.Apply this diff to fix the issue:
- const rawArgv = [...process.argv.slice(2)]; + const rawArgv = process.argv.slice(2);
71-73: Remove unused function.The
logInfofunction is defined but never used in the code.Remove the unused function:
-const logInfo = (message: string) => { - process.stdout.write(`${bold}${message}${reset}\n`); -};
636-636: Remove unused variable.The
fileListvariable is created but never used.Remove the unused variable:
const form = new FormData(); form.append("email", normalizedEmail); -const fileList = positionalContent.files.map(file => file.filename); form.append("message", messageBody);
674-674: Remove unused loop variable.The
namevariable in the loop is never used.Apply this diff to fix the issue:
-for (const [name, interfaces] of networkInterfaces) { +for (const [_name, interfaces] of networkInterfaces) {
505-530: Simplify IP support detection logic.The switch statements in
getIPSupportcontain redundant cases. Several branches return the same value.Simplify the logic:
function getIPSupport(networkInterface: os.NetworkInterfaceInfo, original: IPSupport): IPSupport { if (networkInterface.family === "IPv4") { - switch (original) { - case IPSupport.none: - return IPSupport.ipv4; - case IPSupport.ipv4: - return IPSupport.ipv4_and_ipv6; - case IPSupport.ipv6: - return IPSupport.ipv4_and_ipv6; - case IPSupport.ipv4_and_ipv6: - return IPSupport.ipv4_and_ipv6; - } + return original === IPSupport.none ? IPSupport.ipv4 : + original === IPSupport.ipv4 ? IPSupport.ipv4 : IPSupport.ipv4_and_ipv6; } else if (networkInterface.family === "IPv6") { - switch (original) { - case IPSupport.none: - return IPSupport.ipv6; - case IPSupport.ipv4: - return IPSupport.ipv4_and_ipv6; - case IPSupport.ipv6: - return IPSupport.ipv4_and_ipv6; - case IPSupport.ipv4_and_ipv6: - return IPSupport.ipv4_and_ipv6; - } + return original === IPSupport.none ? IPSupport.ipv6 : + original === IPSupport.ipv6 ? IPSupport.ipv6 : IPSupport.ipv4_and_ipv6; } return original; }
649-649: Add null safety for process.release.sourceUrl.The non-null assertion on
process.release.sourceUrlcould cause runtime errors if the property is undefined.Apply this diff to add proper null safety:
-form.append("bunBuild", path.basename(process.release.sourceUrl!, path.extname(process.release.sourceUrl!))); +const sourceUrl = process.release.sourceUrl || ""; +form.append("bunBuild", sourceUrl ? path.basename(sourceUrl, path.extname(sourceUrl)) : "unknown");src/codegen/bundle-modules.ts (1)
545-564: Consider extracting eval bundling configuration.The eval bundling step duplicates the bundling configuration from the main pipeline. Consider extracting shared configuration to reduce duplication.
Consider extracting the common bundling options:
const commonBundleOptions = { minify: !debug, target: "bun" as const, format: "esm" as const, env: "disable" as const, define: { "process.platform": JSON.stringify(process.platform), "process.arch": JSON.stringify(process.arch), }, }; // Then use it in both places: const evalFiles = new Bun.Glob(path.join(BASE, "eval", "*.ts")).scanSync(); for (const file of evalFiles) { const { outputs: [output], } = await Bun.build({ entrypoints: [file], ...commonBundleOptions, }); writeIfNotChanged(path.join(CODEGEN_DIR, "eval", path.basename(file)), await output.text()); }src/cli/run_command.zig (1)
1608-1608: Function should not return after noreturn call.The function
@"bun feedback"is marked as!noreturnand exits the process, so the try expression is unnecessary.Apply this diff to properly handle the noreturn function:
- try @"bun feedback"(ctx); + @"bun feedback"(ctx) catch |err| { + Output.prettyErrorln("<r><red>error<r>: Failed to launch feedback due to error <b>{s}<r>", .{@errorName(err)}); + Global.exit(1); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
build.zig(1 hunks)src/cli/run_command.zig(2 hunks)src/codegen/bundle-modules.ts(1 hunks)src/js/eval/README.md(1 hunks)src/js/eval/feedback.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/js/eval/README.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
build.zigsrc/cli/run_command.zig
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
src/codegen/bundle-modules.tssrc/js/eval/feedback.ts
src/js/**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
src/js/**/*.{js,ts}: In built-in modules (src/js), require() must use string literals resolved at compile time
In built-in modules (src/js), use export default (converted to a return statement by the preprocessor)
Files:
src/js/eval/feedback.ts
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/cli/run_command.zig
🧠 Learnings (8)
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
build.zig
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/**/js_*.zig : Implement JavaScript bindings in a Zig file named with a js_ prefix (e.g., js_smtp.zig, js_your_feature.zig)
Applied to files:
build.zig
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: For platform-specific changes, run bun run zig:check-all to compile on all platforms
Applied to files:
build.zig
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
src/codegen/bundle-modules.ts
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Follow the build pipeline: Source TS/JS → Preprocessor → Bundler → C++ Headers; IDs assigned A–Z; `$` replaced with `__intrinsic__`; `require("x")` replaced with `$requireId(n)`; `export default` converted to `return`; `__intrinsic__` replaced with `@`; inlined into C++; modules loaded by numeric ID
Applied to files:
src/codegen/bundle-modules.ts
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Author modules as CommonJS-style with `require(...)` and export via `export default {}` (no ESM `import`/named exports)
Applied to files:
src/codegen/bundle-modules.ts
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/bun/**/*.{js,ts} : Place Bun-specific modules (e.g., `bun:ffi`, `bun:sqlite`) under `bun/`
Applied to files:
src/codegen/bundle-modules.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
src/codegen/bundle-modules.ts
🧬 Code graph analysis (1)
src/js/eval/feedback.ts (3)
scripts/glob-sources.mjs (2)
bold(46-46)green(44-44)src/js/node/fs.ts (1)
closeSync(1108-1113)src/js/thirdparty/undici.js (1)
FormData(14-14)
🪛 GitHub Check: Lint JavaScript
src/js/eval/feedback.ts
[failure] 547-547: eslint-plugin-unicorn(no-useless-spread)
Using a spread operator here creates a new array unnecessarily.
[failure] 674-674: eslint(no-unused-vars)
Variable 'name' is declared but never used. Unused variables should start with a '_'.
[failure] 636-636: eslint(no-unused-vars)
Variable 'fileList' is declared but never used. Unused variables should start with a '_'.
[failure] 74-74: eslint(no-unused-vars)
Variable 'logInfo' is declared but never used. Unused variables should start with a '_'.
[failure] 20-20: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
[failure] 19-19: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
[failure] 18-18: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
🔇 Additional comments (3)
build.zig (1)
742-742: LGTM! Clean integration of the feedback module.The addition of the eval/feedback.ts entry follows the existing pattern correctly and uses the appropriate embedding condition.
src/cli/run_command.zig (1)
1606-1610: Remove duplicate auto-run check for feedback command.The auto-run check for the feedback command appears twice (lines 1606-1610 and 1675-1686) with identical logic. The second one is unreachable since execution would have exited on the first match.
Remove the duplicate unreachable block at lines 1675-1686:
- fn @"bun feedback"(ctx: Command.Context) !noreturn { - const trigger = bun.pathLiteral("/[eval]"); - var entry_point_buf: [bun.MAX_PATH_BYTES + trigger.len]u8 = undefined; - const cwd = try std.posix.getcwd(&entry_point_buf); - @memcpy(entry_point_buf[cwd.len..][0..trigger.len], trigger); - ctx.runtime_options.eval.script = if (bun.Environment.codegen_embed) - @embedFile("eval/feedback.ts") - else - bun.runtimeEmbedFile(.codegen, "eval/feedback.ts"); - try Run.boot(ctx, entry_point_buf[0 .. cwd.len + trigger.len], null); - Global.exit(0); - }Also applies to: 1675-1686
⛔ Skipped due to learnings
Learnt from: RiskyMH PR: oven-sh/bun#22613 File: src/cli/package_manager_command.zig:0-0 Timestamp: 2025-09-12T22:27:19.572Z Learning: In Bun's CLI architecture, commands that go through the standard Command.Context flow should use `ctx.positionals` (which excludes flags) rather than manually parsing `bun.argv`. Manual `bun.argv` parsing is only used for standalone commands like `bun info` that bypass the normal command routing.src/js/eval/feedback.ts (1)
659-661: Use a static top-level fs requiresrc/js/eval/feedback.ts:659 uses require("fs") inline — change to a top-level const fs = require("fs") (or const fsp = require("fs").promises and use fsp.access(...) inside an async context) to avoid bundling/runtime issues. Confirm whether the eval runtime/bundler accepts inline requires.
| question: `${cyan}?${reset}` || "?", | ||
| check: `${green}✔${reset}` || "✔", | ||
| cross: `${red}✖${reset}` || "✖", |
There was a problem hiding this comment.
Fix logical errors in symbol definitions.
The logical OR operators with string literals will always return the first truthy value (the ANSI escape sequence strings), making the fallback characters unreachable.
Apply this diff to fix the logic:
const symbols = {
- question: `${cyan}?${reset}` || "?",
- check: `${green}✔${reset}` || "✔",
- cross: `${red}✖${reset}` || "✖",
+ question: supportsAnsi ? `${cyan}?${reset}` : "?",
+ check: supportsAnsi ? `${green}✔${reset}` : "✔",
+ cross: supportsAnsi ? `${red}✖${reset}` : "✖",
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| question: `${cyan}?${reset}` || "?", | |
| check: `${green}✔${reset}` || "✔", | |
| cross: `${red}✖${reset}` || "✖", | |
| const symbols = { | |
| question: supportsAnsi ? `${cyan}?${reset}` : "?", | |
| check: supportsAnsi ? `${green}✔${reset}` : "✔", | |
| cross: supportsAnsi ? `${red}✖${reset}` : "✖", | |
| }; |
🧰 Tools
🪛 GitHub Check: Lint JavaScript
[failure] 20-20: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
[failure] 19-19: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
[failure] 18-18: eslint(no-constant-binary-expression)
Unexpected constant truthiness on the left-hand side of a "||" expression
🤖 Prompt for AI Agents
In src/js/eval/feedback.ts around lines 18 to 20, the use of || with the
template literals makes the fallbacks unreachable because the template strings
are always truthy; replace each OR-expression with a conditional that checks
whether the color code is present (e.g., use cyan ? `${cyan}?${reset}` : "?" for
question, green ? `${green}✔${reset}` : "✔" for check, and red ?
`${red}✖${reset}` : "✖") so the plain characters are used when color codes are
not available.
| // Truncate to | ||
| if (fileContents.length > 1024 * 1024 * 10) { | ||
| fileContents = fileContents.slice(0, 1024 * 1024 * 10); | ||
| } |
There was a problem hiding this comment.
Improve file size handling and add proper documentation.
The 10MB file truncation lacks proper documentation and the comment on line 485 is incomplete. This could confuse users when their files are silently truncated.
Apply this diff to improve the implementation:
let fileContents = await Bun.file(filePath).bytes();
-// Truncate to
+// Truncate files larger than 10MB to prevent excessive upload sizes
+const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
-if (fileContents.length > 1024 * 1024 * 10) {
- fileContents = fileContents.slice(0, 1024 * 1024 * 10);
+if (fileContents.length > MAX_FILE_SIZE) {
+ console.warn(`File ${filePath} exceeds 10MB limit and will be truncated`);
+ fileContents = fileContents.slice(0, MAX_FILE_SIZE);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Truncate to | |
| if (fileContents.length > 1024 * 1024 * 10) { | |
| fileContents = fileContents.slice(0, 1024 * 1024 * 10); | |
| } | |
| let fileContents = await Bun.file(filePath).bytes(); | |
| // Truncate files larger than 10MB to prevent excessive upload sizes | |
| const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB | |
| if (fileContents.length > MAX_FILE_SIZE) { | |
| console.warn(`File ${filePath} exceeds 10MB limit and will be truncated`); | |
| fileContents = fileContents.slice(0, MAX_FILE_SIZE); | |
| } |
🤖 Prompt for AI Agents
In src/js/eval/feedback.ts around lines 485 to 488, the current inline
truncation silently cuts files to 10MB with an incomplete comment; replace this
with a documented named constant (e.g., MAX_FILE_BYTES = 10 * 1024 * 1024),
update the comment to explain why files are truncated and that only the first
MAX_FILE_BYTES are kept, and add a non-silent indicator (either a log warning or
a metadata flag on the file object) when truncation occurs so callers/users can
detect that the content was truncated; ensure you use the constant in the slice
call and update any related types/docs to reflect the truncation behavior.
| question: `${cyan}?${reset}` || "?", | ||
| check: `${green}✔${reset}` || "✔", | ||
| cross: `${red}✖${reset}` || "✖", |
There was a problem hiding this comment.
| question: `${cyan}?${reset}` || "?", | |
| check: `${green}✔${reset}` || "✔", | |
| cross: `${red}✖${reset}` || "✖", | |
| question: `${cyan}?${reset}`, | |
| check: `${green}✔${reset}`, | |
| cross: `${red}✖${reset}`, |
Since reset, cyan, etc. are already conditional on supportsAnsi.
What does this PR do?
How did you verify your code works?