refactor: replace process.stdin with Bun.stdin.json() - #47
johnlindquist wants to merge 18 commits into
Conversation
- Update runHook to use await Bun.stdin.json() for cleaner JSON parsing - Make runHook function async and update callers - Add comprehensive unit tests for stdin handling - Update smoke tests to check for new implementation This change modernizes the stdin handling to use Bun's built-in JSON parsing capabilities, removing the need for manual stream handling and JSON parsing.
WalkthroughThe changes update the stdin handling in hook scripts from a Node.js-style event-based approach to an asynchronous Bun-specific method using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HookScript
participant Bun
User->>HookScript: Start process
HookScript->>Bun: await Bun.stdin.json()
Bun-->>HookScript: JSON input
HookScript->>HookScript: Process input, invoke handler
HookScript-->>User: Output JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
templates/hooks/index.ts(1 hunks)templates/hooks/lib.ts(1 hunks)test/smoke/generated-files.test.ts(1 hunks)test/unit/stdin-handling.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
templates/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Hook templates must be stored in the
templates/directory and copied to the user's.claude/directory when initialized.
Files:
templates/hooks/index.tstemplates/hooks/lib.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test files should be organized and run using the provided bun scripts: `bun test`, `bun run test:unit`, `bun run test:integration`, `bun run test:smoke`, `bun run test:coverage`.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., `bun run build`, `bun run lint`, `bun run format`, `bun run clean`).
templates/hooks/index.ts (2)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
test/smoke/generated-files.test.ts (4)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test with a tsconfig.json in the current directory.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
test/unit/stdin-handling.test.ts (6)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test with a tsconfig.json in the current directory.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., bun run build, bun run lint, bun run format, bun run clean).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Test from different directories to catch path-related issues.
templates/hooks/lib.ts (3)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.825Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., bun run build, bun run lint, bun run format, bun run clean).
🧬 Code Graph Analysis (1)
templates/hooks/lib.ts (1)
src/commands/init.ts (1)
updateSettings(221-318)
🪛 GitHub Actions: test
test/unit/stdin-handling.test.ts
[error] 1-1: TypeScript error TS2307: Cannot find module 'bun:test' or its corresponding type declarations.
🔇 Additional comments (7)
templates/hooks/lib.ts (3)
149-149: LGTM: Function signature correctly updated to async.The
runHookfunction signature is properly updated to returnPromise<void>, reflecting the asynchronous nature of the new stdin handling.
152-158: Excellent refactoring to use Bun's native JSON parsing.The change from manual event-based
process.stdinhandling toawait Bun.stdin.json()is a significant improvement:
- Cleaner, more readable code
- Leverages Bun's optimized JSON parsing
- Eliminates manual stream handling and string concatenation
- Better aligns with Bun's modern async patterns
229-232: Good error handling with appropriate fallback.The try-catch block properly handles malformed JSON input and provides a sensible default response (
action: 'continue') to prevent hook failures from blocking the system.templates/hooks/index.ts (1)
145-145: LGTM: Correctly added await for async runHook call.The addition of
awaitis necessary sincerunHooknow returns aPromise<void>. This ensures proper asynchronous execution flow.test/smoke/generated-files.test.ts (1)
154-157: LGTM: Test expectation correctly updated for new stdin handling.The test assertion properly validates that the generated
lib.tsfile uses the newawait Bun.stdin.json()approach instead of the previous event-basedprocess.stdinhandling. This ensures the smoke test stays in sync with the implementation changes.test/unit/stdin-handling.test.ts (2)
24-89: Excellent test coverage for basic JSON stdin handling.This test thoroughly validates the new
Bun.stdin.json()functionality:
- Creates a realistic hook script that uses the new API
- Tests actual process execution with stdin/stdout communication
- Verifies correct JSON parsing and response generation
- Includes proper error handling for process failures
The test design is robust and provides confidence in the refactoring.
91-154: Great test for large payload handling.This test ensures the new stdin handling can manage large JSON payloads (100KB), which is important for hook systems that might receive substantial data. The test structure mirrors the first test with appropriate modifications for the large payload scenario.
| import {afterEach, beforeEach, describe, expect, it} from 'bun:test' | ||
| import {spawn} from 'child_process' | ||
| import {promises as fs} from 'fs' | ||
| import os from 'os' | ||
| import path from 'path' | ||
|
|
There was a problem hiding this comment.
Fix the TypeScript import error for bun:test.
The pipeline is failing because TypeScript cannot find the bun:test module types. This is likely because the test environment doesn't have the proper Bun type definitions.
Based on the retrieved learnings that indicate tests should use the provided bun scripts, verify that the test environment has the correct Bun setup:
#!/bin/bash
# Check if @types/bun is installed and bun test types are available
cd "$(git rev-parse --show-toplevel)"
bun --version
ls -la node_modules/@types/bun/ 2>/dev/null || echo "No @types/bun found"
bun test --help | head -5🧰 Tools
🪛 GitHub Actions: test
[error] 1-1: TypeScript error TS2307: Cannot find module 'bun:test' or its corresponding type declarations.
🤖 Prompt for AI Agents
In test/unit/stdin-handling.test.ts at the beginning of the file, the import
from 'bun:test' causes a TypeScript error due to missing Bun type definitions.
To fix this, ensure the test environment is correctly set up with Bun's type
definitions by verifying that @types/bun is installed and accessible. Run the
provided shell commands to check Bun's version, confirm the presence of
@types/bun in node_modules, and validate that 'bun test' is properly configured.
Adjust the environment or install missing types so TypeScript can resolve
'bun:test' module types without errors.
- Use unique temp directories for each test to avoid Windows file locking issues - Add timeout and windowsHide options to all execSync calls - Fix duplicate object properties in test configuration - Update CI workflow to use 'bun test' directly instead of 'npm test' - Remove unreachable code after this.skip() calls
- Increase execSync timeout from 10s to 30s for all tests - Add better error logging in the 'runs setup' test - Set explicit stdio options to prevent hanging
- Set 30s timeout for all tests to prevent Windows timeouts - Ensure GitHub Actions already has the timeout flag
- Add better error logging to stdin tests to debug Windows issues - Change permission test skip approach for bun test compatibility - Add check for empty output before JSON parsing
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
examples/pretooluse-hook-examples.md (2)
293-307: Remove trailing colons from section headingsMarkdown-lint (MD026) flags headings that end with punctuation. Deleting the colons keeps the headings valid and cleaner.
-### Common patterns observed: +### Common patterns observed -### Best practices identified: +### Best practices identified -### Recommendations for implementation: +### Recommendations for implementation
104-109: Tighten wording in bullet pointsLanguage-Tool highlights the phrase “with success/failure status” as wordy. Switching to a more concise wording improves readability.
- Returns structured HookResult objects with success/failure status and exit codes + Returns structured HookResult objects containing status flags and exit codes - Hooks return standardized result objects with success/failure status + Hooks return standardized result objects that include status flagsAlso applies to: 296-297
.github/workflows/test.yml (1)
69-69: Add missing newline at end of file.The YAML file is missing a newline character at the end, which violates YAML formatting standards.
Apply this fix:
- - run: bun test --timeout 30000 test/integration + - run: bun test --timeout 30000 test/integration +package.json (1)
93-93: Add missing newline at end of file.The JSON file is missing a newline character at the end, which is a formatting best practice.
Apply this fix:
} +.githooks/pre-push (1)
16-16: Consider using the package.json test script for consistency.While
bun testworks correctly, usingnpm test(which now runsbun test --timeout 30000) would be more consistent with the package.json scripts and provide the same timeout configuration used in CI.Apply this change for consistency:
-bun test +npm testThis ensures the same timeout and configuration as defined in package.json and used in CI workflows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.githooks/README.md(1 hunks).githooks/pre-push(1 hunks).github/workflows/test.yml(3 hunks)examples/pretooluse-hook-examples.md(1 hunks)package.json(1 hunks)test/commands/init.test.ts(16 hunks)test/unit/stdin-handling.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .githooks/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/stdin-handling.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
package.json
📄 CodeRabbit Inference Engine (CLAUDE.md)
The project must use ESM modules (
"type": "module").
Files:
package.json
{package.json,src/commands/**/*}
📄 CodeRabbit Inference Engine (CLAUDE.md)
When improving CLI help, update the package.json description to match README, update command descriptions in the static description field, run
npx oclif manifestafter changes, and consider removing irrelevant plugins.
Files:
package.json
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Hooks are executed using the Bun runtime (required dependency).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., `bun run build`, `bun run lint`, `bun run format`, `bun run clean`).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test files should be organized and run using the provided bun scripts: `bun test`, `bun run test:unit`, `bun run test:integration`, `bun run test:smoke`, `bun run test:coverage`.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create `.claude/settings.json`, `.claude/hooks/index.ts`, `.claude/hooks/lib.ts`, and `.claude/hooks/session.ts`.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to {package.json,src/commands/**/*} : When improving CLI help, update the package.json description to match README, update command descriptions in the static description field, run `npx oclif manifest` after changes, and consider removing irrelevant plugins.
package.json (8)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., bun run build, bun run lint, bun run format, bun run clean).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to {package.json,src/commands/**/*} : When improving CLI help, update the package.json description to match README, update command descriptions in the static description field, run npx oclif manifest after changes, and consider removing irrelevant plugins.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to package.json : The project must use ESM modules ("type": "module").
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to bin/run.js : Use NODE_ENV=production to affect behavior during import time.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test with a tsconfig.json in the current directory.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Always test with the actual published npm package using npx package-name@latest before declaring a fix complete.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to bin/run.js : Set NODE_ENV=production in bin/run.js BEFORE importing @oclif/core to prevent TypeScript detection warnings.
.githooks/pre-push (3)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Ensure all existing tests pass before pushing.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., bun run build, bun run lint, bun run format, bun run clean).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
.github/workflows/test.yml (4)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Commands for building, linting, formatting, and cleaning should use the provided bun scripts (e.g., bun run build, bun run lint, bun run format, bun run clean).
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Ensure all existing tests pass before pushing.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test with a tsconfig.json in the current directory.
test/commands/init.test.ts (7)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test with a tsconfig.json in the current directory.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test from different directories to catch path-related issues.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to {package.json,src/commands/**/*} : When improving CLI help, update the package.json description to match README, update command descriptions in the static description field, run npx oclif manifest after changes, and consider removing irrelevant plugins.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to bin/run.js : Set NODE_ENV=production in bin/run.js BEFORE importing @oclif/core to prevent TypeScript detection warnings.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Test files should be organized and run using the provided bun scripts: bun test, bun run test:unit, bun run test:integration, bun run test:smoke, bun run test:coverage.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to bin/run.js : Use NODE_ENV=production to affect behavior during import time.
examples/pretooluse-hook-examples.md (2)
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to .claude/{settings.json,hooks/index.ts,hooks/lib.ts,hooks/session.ts} : The generated hook system must create .claude/settings.json, .claude/hooks/index.ts, .claude/hooks/lib.ts, and .claude/hooks/session.ts.
Learnt from: CR
PR: johnlindquist/claude-hooks#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T15:51:09.840Z
Learning: Applies to templates/**/* : Hook templates must be stored in the templates/ directory and copied to the user's .claude/ directory when initialized.
🪛 YAMLlint (1.37.1)
.github/workflows/test.yml
[error] 69-69: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
examples/pretooluse-hook-examples.md
[style] ~108-~108: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...- Returns structured HookResult objects with success/failure status and exit codes - Handles...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[style] ~296-~296: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...ooks return standardized result objects with success/failure status 4. Error Handling: C...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
🪛 markdownlint-cli2 (0.17.2)
examples/pretooluse-hook-examples.md
293-293: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
300-300: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
307-307: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (9)
.github/workflows/test.yml (1)
28-28: LGTM! Test runner migration to Bun is consistent.The switch from npm test commands to
bun test --timeout 30000aligns perfectly with the project's migration from mocha to Bun's test runner. The 30-second timeout is appropriate for the test execution requirements.Also applies to: 54-54, 69-69
package.json (1)
83-86: LGTM! Consistent migration to Bun test runner.The migration from mocha to
bun testacross all test scripts is well-executed:
- Main test with 30-second timeout
- Unit and command tests without timeout (appropriate for faster tests)
- Integration and smoke tests with 10-second timeout (balanced for CI performance)
This aligns perfectly with the retrieved learnings about using bun scripts for testing.
.githooks/pre-push (1)
1-23: LGTM! Well-structured pre-push hook with quality gates.The hook effectively enforces code quality by running linting and tests before allowing pushes. The error handling and user feedback are well-implemented.
test/commands/init.test.ts (6)
7-7: Good addition for cross-platform compatibility.Adding the
osimport enables proper temp directory handling for Windows compatibility.
17-19: Excellent improvement for test isolation.Using
fs.mkdtemp()with the system temp directory creates unique test directories, preventing test interference and improving reliability across different environments.
22-35: Robust Windows-compatible cleanup implementation.The retry mechanism for Windows
EBUSYerrors is a practical solution to filesystem locking issues. The graceful error handling with the 100ms delay and final error suppression ensures tests don't fail due to cleanup issues.
47-51: Comprehensive Windows compatibility and timeout improvements.The addition of
timeout: 30000andwindowsHide: trueto allexecSynccalls significantly improves cross-platform reliability:
- The 30-second timeout aligns with the bun test runner migration
windowsHide: trueprevents console window flashing on Windows- Explicit
stdioconfiguration in some calls ensures proper output handlingThese changes demonstrate thorough attention to Windows compatibility issues.
Also applies to: 62-68, 84-85, 99-100, 144-145, 180-181, 192-193, 200-201, 213-214, 220-221, 233-234, 246-247, 272-273, 280-281, 301-302, 320-321, 332-333, 340-341, 366-367
71-77: Enhanced error diagnostics for debugging.The detailed error logging with stdout/stderr output will significantly help with debugging test failures, especially in CI environments where detailed error information is crucial.
352-355: Appropriate platform-specific test exclusion.Excluding the permission error test on Windows is correct since file permissions work differently on Windows. This prevents false test failures while maintaining coverage on Unix-like systems.
1 participant
Summary
This PR refactors the stdin handling in the hooks system to use Bun's built-in JSON parsing capabilities.
Changes
process.stdin.on('data', ...)withawait Bun.stdin.json()intemplates/hooks/lib.tsrunHookfunction to be asynctemplates/hooks/index.tsto await the asyncrunHookcallBenefits
Testing
Technical Details
The change modernizes the stdin handling from:
To:
This simplifies the code and leverages Bun's native capabilities for better performance and reliability.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Documentation