Skip to content

Commit 559e121

Browse files
retsohuangclaude
andcommitted
fix: address GitHub Copilot review feedback for isomorphic-git migration
- Fix Node.js fs import to use promises-based interface with sync methods - Fix memfs Volume type to properly implement PromiseFsClient - Fix test setup order - create directory before git init - Fix branch name expectation from 'master' to 'main' - Fix main execution guard to use reliable endsWith check - Fix README table formatting (remove extra pipe characters) - Fix loadConfig call to use correct parameter order - Add defaultBranch: 'main' to git init in tests All tests now pass successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f6027ed commit 559e121

4 files changed

Lines changed: 56 additions & 31 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ The marketplace is configured in `.claude-plugin/marketplace.json`. Key fields:
134134
## Available Plugins
135135

136136
| Plugin Name | Description | Version |
137-
|-------------|-------------|---------|||
137+
|-------------|-------------|---------|
138138
| [code-review](./plugins/code-review/README.md) | Review code changes commit-by-commit with custom rules support. Includes interactive setup and rule creation. | 2.1.0 |
139139

140140
## Contributing

plugins/code-review-tools/scripts/dist/cli.js

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17004,7 +17004,7 @@ gpgsig`));
1700417004

1700517005
// src/cli.ts
1700617006
var import_isomorphic_git = __toESM(require_isomorphic_git(), 1);
17007-
import nodeFs from "node:fs";
17007+
import * as nodeFs from "node:fs";
1700817008
import { join } from "node:path";
1700917009

1701017010
// node_modules/zod/v4/classic/external.js
@@ -29632,26 +29632,32 @@ function parseConfig(input) {
2963229632

2963329633
// src/cli.ts
2963429634
var DEFAULT_DIR = process.cwd();
29635+
var defaultFs = {
29636+
promises: nodeFs.promises,
29637+
existsSync: nodeFs.existsSync,
29638+
readFileSync: nodeFs.readFileSync,
29639+
writeFileSync: nodeFs.writeFileSync,
29640+
mkdirSync: nodeFs.mkdirSync
29641+
};
2963529642
function success2(data) {
2963629643
return { success: true, data };
2963729644
}
2963829645
function error46(message) {
2963929646
return { success: false, error: message };
2964029647
}
29641-
function loadConfig(configPath, fs = nodeFs) {
29642-
const path = configPath || ".claude/code-review-tools/config.json";
29648+
function loadConfig(configPath = ".claude/code-review-tools/config.json", fs = defaultFs) {
2964329649
try {
29644-
if (!fs.existsSync(path)) {
29650+
if (!fs.existsSync(configPath)) {
2964529651
return success2(DEFAULT_CONFIG);
2964629652
}
29647-
const userConfig = JSON.parse(fs.readFileSync(path, "utf-8"));
29653+
const userConfig = JSON.parse(fs.readFileSync(configPath, "utf-8"));
2964829654
const config2 = parseConfig(userConfig);
2964929655
return success2(config2);
2965029656
} catch (err) {
2965129657
return error46(`Failed to load config: ${err.message}`);
2965229658
}
2965329659
}
29654-
async function collectCommits(commitHash, dir = DEFAULT_DIR, fs = nodeFs) {
29660+
async function collectCommits(commitHash, dir = DEFAULT_DIR, fs = defaultFs) {
2965529661
try {
2965629662
const branch = await import_isomorphic_git.currentBranch({ fs, dir }) ?? "HEAD";
2965729663
const allCommits = await import_isomorphic_git.log({ fs, dir, ref: "HEAD" });
@@ -29695,7 +29701,7 @@ async function collectCommits(commitHash, dir = DEFAULT_DIR, fs = nodeFs) {
2969529701
return error46(`Failed to collect commits: ${err.message}`);
2969629702
}
2969729703
}
29698-
function buildRules(config2, pluginRoot, fs = nodeFs) {
29704+
function buildRules(config2, pluginRoot, fs = defaultFs) {
2969929705
try {
2970029706
const rulesSections = [];
2970129707
let enabledCount = 0;
@@ -29760,7 +29766,7 @@ ${content}
2976029766
return error46(`Failed to build rules: ${err.message}`);
2976129767
}
2976229768
}
29763-
function loadTemplate(customTemplate, pluginRoot, defaultTemplateName, fs = nodeFs) {
29769+
function loadTemplate(customTemplate, pluginRoot, defaultTemplateName, fs = defaultFs) {
2976429770
let templatePath;
2976529771
if (customTemplate) {
2976629772
templatePath = `.claude/code-review-tools/templates/${customTemplate}`;
@@ -29777,7 +29783,7 @@ function loadTemplate(customTemplate, pluginRoot, defaultTemplateName, fs = node
2977729783
}
2977829784
throw new Error(`Template not found: ${defaultPath}`);
2977929785
}
29780-
async function prepareReview(commitHash, pluginRoot, dir = DEFAULT_DIR, fs = nodeFs) {
29786+
async function prepareReview(commitHash, pluginRoot, dir = DEFAULT_DIR, fs = defaultFs) {
2978129787
try {
2978229788
const configResult = loadConfig(undefined, fs);
2978329789
if (!configResult.success) {
@@ -29879,7 +29885,7 @@ async function main() {
2987929885
process.exit(1);
2988029886
}
2988129887
}
29882-
if (import.meta.url === `file://${process.argv[1]}`) {
29888+
if (import.meta.url.endsWith(process.argv[1])) {
2988329889
main().catch((err) => {
2988429890
console.error("Fatal error:", err.message);
2988529891
process.exit(1);

plugins/code-review-tools/scripts/src/cli.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
readCommit,
77
type PromiseFsClient,
88
} from "isomorphic-git"
9-
import nodeFs from "node:fs"
9+
import * as nodeFs from "node:fs"
1010
import { join } from "node:path"
1111
import {
1212
DEFAULT_CONFIG,
@@ -19,8 +19,18 @@ const DEFAULT_DIR = process.cwd()
1919
interface FsLike extends PromiseFsClient {
2020
existsSync(path: string): boolean
2121
readFileSync(path: string, encoding: BufferEncoding): string
22+
writeFileSync(path: string, data: string): void
23+
mkdirSync(path: string, options?: { recursive?: boolean }): void
2224
}
2325

26+
const defaultFs: FsLike = {
27+
promises: nodeFs.promises,
28+
existsSync: nodeFs.existsSync,
29+
readFileSync: nodeFs.readFileSync,
30+
writeFileSync: nodeFs.writeFileSync,
31+
mkdirSync: nodeFs.mkdirSync,
32+
} as FsLike
33+
2434
interface SuccessOutput<T = unknown> {
2535
success: true
2636
data: T
@@ -42,17 +52,15 @@ function error(message: string): ErrorOutput {
4252
}
4353

4454
function loadConfig(
45-
configPath?: string,
46-
fs: FsLike = nodeFs,
55+
configPath: string = ".claude/code-review-tools/config.json",
56+
fs: FsLike = defaultFs,
4757
): Output<ReviewConfig> {
48-
const path = configPath || ".claude/code-review-tools/config.json"
49-
5058
try {
51-
if (!fs.existsSync(path)) {
59+
if (!fs.existsSync(configPath)) {
5260
return success(DEFAULT_CONFIG)
5361
}
5462

55-
const userConfig = JSON.parse(fs.readFileSync(path, "utf-8"))
63+
const userConfig = JSON.parse(fs.readFileSync(configPath, "utf-8"))
5664
const config = parseConfig(userConfig)
5765

5866
return success(config)
@@ -79,7 +87,7 @@ interface CollectCommitsResult {
7987
async function collectCommits(
8088
commitHash: string,
8189
dir: string = DEFAULT_DIR,
82-
fs: FsLike = nodeFs,
90+
fs: FsLike = defaultFs,
8391
): Promise<Output<CollectCommitsResult>> {
8492
try {
8593
const branch = (await currentBranch({ fs, dir })) ?? "HEAD"
@@ -141,7 +149,7 @@ interface BuildRulesResult {
141149
function buildRules(
142150
config: ReviewConfig,
143151
pluginRoot: string,
144-
fs: FsLike = nodeFs,
152+
fs: FsLike = defaultFs,
145153
): Output<BuildRulesResult> {
146154
try {
147155
const rulesSections: string[] = []
@@ -219,7 +227,7 @@ function loadTemplate(
219227
customTemplate: string | undefined,
220228
pluginRoot: string,
221229
defaultTemplateName: string,
222-
fs: FsLike = nodeFs,
230+
fs: FsLike = defaultFs,
223231
): string {
224232
let templatePath: string
225233

@@ -246,7 +254,7 @@ async function prepareReview(
246254
commitHash: string,
247255
pluginRoot: string,
248256
dir: string = DEFAULT_DIR,
249-
fs: FsLike = nodeFs,
257+
fs: FsLike = defaultFs,
250258
): Promise<Output<PrepareReviewResult>> {
251259
try {
252260
const configResult = loadConfig(undefined, fs)
@@ -397,7 +405,7 @@ async function main(): Promise<void> {
397405
}
398406
}
399407

400-
if (import.meta.url === `file://${process.argv[1]}`) {
408+
if (import.meta.url.endsWith(process.argv[1])) {
401409
main().catch((err) => {
402410
console.error("Fatal error:", (err as Error).message)
403411
process.exit(1)

plugins/code-review-tools/scripts/tests/cli.test.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, test } from "bun:test"
22
import { add, commit, init, setConfig } from "isomorphic-git"
3-
import { Volume } from "memfs"
3+
import { Volume, createFsFromVolume } from "memfs"
44
import { join } from "node:path"
55
import {
66
collectCommits,
@@ -11,23 +11,34 @@ import {
1111

1212
const PLUGIN_ROOT = join(__dirname, "../../")
1313

14-
let fs: FsLike & Volume
14+
let fs: FsLike
15+
let vol: Volume
1516
let testCommits: string[] = []
1617
const TEST_DIR = "/repo"
1718

1819
beforeEach(async () => {
19-
fs = new Volume() as FsLike & Volume
20+
vol = new Volume()
21+
const memfsInstance = createFsFromVolume(vol)
2022

21-
await init({ fs, dir: TEST_DIR })
23+
// Create FsLike by combining memfs with sync methods
24+
fs = {
25+
promises: memfsInstance.promises,
26+
existsSync: memfsInstance.existsSync.bind(memfsInstance),
27+
readFileSync: memfsInstance.readFileSync.bind(memfsInstance),
28+
writeFileSync: memfsInstance.writeFileSync.bind(memfsInstance),
29+
mkdirSync: memfsInstance.mkdirSync.bind(memfsInstance),
30+
} as FsLike
31+
32+
vol.mkdirSync(TEST_DIR, { recursive: true })
33+
34+
await init({ fs, dir: TEST_DIR, defaultBranch: "main" })
2235
await setConfig({
2336
fs,
2437
dir: TEST_DIR,
2538
path: "user.email",
2639
value: "test@example.com",
2740
})
2841
await setConfig({ fs, dir: TEST_DIR, path: "user.name", value: "Test User" })
29-
30-
fs.mkdirSync(TEST_DIR, { recursive: true })
3142
fs.writeFileSync(join(TEST_DIR, "file1.txt"), "initial content")
3243
await add({ fs, dir: TEST_DIR, filepath: "file1.txt" })
3344
const commit1 = await commit({
@@ -87,7 +98,7 @@ beforeEach(async () => {
8798
})
8899

89100
afterEach(() => {
90-
fs.reset()
101+
vol.reset()
91102
})
92103

93104
describe("collectCommits", () => {
@@ -100,7 +111,7 @@ describe("collectCommits", () => {
100111
if (result.success) {
101112
expect(result.data.totalCommits).toBe(2)
102113
expect(result.data.commits).toHaveLength(2)
103-
expect(result.data.branch).toBe("master")
114+
expect(result.data.branch).toBe("main")
104115
expect(result.data.commitRange).toContain("..")
105116
}
106117
})

0 commit comments

Comments
 (0)