Skip to content

Commit db2f55b

Browse files
JustAGhosTclaude
andcommitted
fix(engine): address four Copilot review comments on PR #433
- vitest.config.mjs: move coverage block inside test:{} so Vitest applies it correctly (top-level coverage is ignored) - check.mjs: detect package manager from package.json#packageManager or lockfiles instead of hardcoding pnpm; npm projects now use 'npm run typecheck', yarn uses 'yarn typecheck', pnpm unchanged - check.mjs: fix warning to print typecheckCmd (the resolved command) rather than stack.typecheck (the config value before resolution) - cli.mjs: add early --help/-h check before ensureDependencies() to avoid installing packages just to print usage text Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 573b2fb commit db2f55b

3 files changed

Lines changed: 35 additions & 6 deletions

File tree

.agentkit/engines/node/src/check.mjs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,27 @@ import { commandExists, execCommand, formatDuration, isValidCommand } from './ru
2323
* @param {string} projectRoot - Project root path
2424
* @returns {string} Command to run
2525
*/
26+
/**
27+
* Detect the package manager in use for a project.
28+
* Checks package.json#packageManager first, then falls back to lockfile detection.
29+
* @param {string} projectRoot
30+
* @param {object} [pkg] - Already-parsed package.json (optional, avoids re-read)
31+
* @returns {'pnpm'|'yarn'|'npm'}
32+
*/
33+
function detectPackageManager(projectRoot, pkg) {
34+
// 1. Explicit declaration in package.json
35+
const pmField = pkg?.packageManager;
36+
if (typeof pmField === 'string') {
37+
if (pmField.startsWith('pnpm')) return 'pnpm';
38+
if (pmField.startsWith('yarn')) return 'yarn';
39+
if (pmField.startsWith('npm')) return 'npm';
40+
}
41+
// 2. Lockfile heuristic
42+
if (existsSync(resolve(projectRoot, 'pnpm-lock.yaml'))) return 'pnpm';
43+
if (existsSync(resolve(projectRoot, 'yarn.lock'))) return 'yarn';
44+
return 'npm';
45+
}
46+
2647
function resolveTypecheckCommand(stack, projectRoot) {
2748
if (stack.name !== 'node' || !stack.typecheck || !projectRoot) return stack.typecheck;
2849
try {
@@ -32,7 +53,9 @@ function resolveTypecheckCommand(stack, projectRoot) {
3253
const script = pkg.scripts?.typecheck;
3354
if (typeof script !== 'string' || !script.trim()) return stack.typecheck;
3455
if (/^node\s+-e\s+/.test(script.trim())) return script.trim();
35-
return 'pnpm typecheck';
56+
const pm = detectPackageManager(projectRoot, pkg);
57+
// pnpm and yarn can run scripts without the `run` sub-command
58+
return pm === 'npm' ? 'npm run typecheck' : `${pm} typecheck`;
3659
} catch {
3760
/* ignore */
3861
}
@@ -89,7 +112,7 @@ function buildSteps(stack, flags, agentkitRoot, projectRoot) {
89112
if (stack.typecheck) {
90113
const typecheckCmd = resolveTypecheckCommand(stack, projectRoot);
91114
if (!isValidCommand(typecheckCmd)) {
92-
console.warn(`[agentkit:check] Skipping invalid typecheck command: ${stack.typecheck}`);
115+
console.warn(`[agentkit:check] Skipping invalid typecheck command: ${typecheckCmd}`);
93116
} else {
94117
steps.push({
95118
name: 'typecheck',

.agentkit/engines/node/src/cli.mjs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,12 @@ async function main() {
492492
process.exit(1);
493493
}
494494

495+
// Early --help check before dependency installation to avoid side effects.
496+
if (commandArgs.includes('--help') || commandArgs.includes('-h')) {
497+
showHelp();
498+
process.exit(0);
499+
}
500+
495501
if (!ensureDependencies(AGENTKIT_ROOT)) {
496502
process.exit(1);
497503
}

.agentkit/vitest.config.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ export default defineConfig({
99
GIT_CONFIG_KEY_0: 'commit.gpgsign',
1010
GIT_CONFIG_VALUE_0: 'false',
1111
},
12-
},
13-
coverage: {
14-
provider: 'v8',
15-
reporter: ['text', 'text-summary'],
12+
coverage: {
13+
provider: 'v8',
14+
reporter: ['text', 'text-summary'],
15+
},
1616
},
1717
});

0 commit comments

Comments
 (0)