Conversation
`bin/*.ts`をNode.jsが直接実行していた構成から、 viteによるバンドル方式に移行します。 外部ライブラリを追加した時の依存解決をビルド時へ寄せて挙動を明示化します。 主な変更内容は以下です。 - TSソースを`bin/`から`src/bin/`に移動し、viteで`dist/bin/*.js`へバンドル - `bin/`には薄いbashラッパーを配置し、`readlink -f`で自身の位置を解決して`dist/bin/*.js`を`exec node` - kyosei同様に`ssr.noExternal: true`で全依存をバンドルに含める - `hooks/hooks.json`と`hooks/build`で`SessionStart`と`PreToolUse`の両方で自動ビルド - `tsconfig.json`を`bundler`/`preserve`に切り替え、importの`.ts`拡張子を全削除 - `editor.ts`はLinuxの`/usr/bin/editor`との衝突を避けて`konoka-editor`にrename - `package.json`の`bin`フィールドは廃止(Claude Codeが`bin/`を自動PATH追加するため不要) - `engines.node`をnixpkgs stableに合わせて`>=22.0.0`に統一 kyoseiは`Bash(node:*)`を広く許可して`node`起動コマンドを`SKILL.md`本文に書きますが、 今回はbashラッパーを噛ませることで個別の`Bash(<name>:*)`許可を維持し、 権限粒度を絞ったまま移行しています。
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
本PRはcommit/prプラグインのTS直接実行をViteバンドル経由に統一する大きめのリファクタです。kyoseiプラグインの構成と揃える方向性は妥当で、bashラッパー+exec nodeの作りも安全に書かれています。security-reviewerは新たな脆弱性を検出していません。
主な指摘は以下です。
ドキュメント整合性 (IMPORTANT)
本PRはdiffに含まれないためインライン化できませんが、READMEのNode.jsバージョン要件が新しい構成と不整合です。両方とも更新をお願いします。
plugins/commit/README.md:Node.js >= 23.6.0(TypeScriptファイルのフラグなし実行に必要)→engines.nodeが>=22.0.0になっており、Vite経由実行に変わったので括弧書きの理由も書き換えてください。plugins/pr/README.md: 同様にNode.js >= 24の記述を>=22.0.0に揃え、理由をViteバンドル成果物の実行に即した内容に更新してください。
コード品質 (WARNING)
konoka-editor.tsのEDITOR環境変数のパースが空白splitのみで、スペース入りパスやクォート付き引数を扱えません(詳細は当該ファイルのインラインコメント参照)。assertEditorSuccessのthrowはトップレベルでcatchされず、他のCLIエントリと挙動が揃いません。plugins/commit/src/bin/konoka-editor.tsとplugins/pr/src/bin/konoka-editor.tsは完全に同一内容のため、共有モジュール化の検討余地があります。
依存方針の不揃い (NOTE)
@types/nodeを22.19.19、engines.nodeを>=22.0.0に揃えた一方、plugins/kyoseiは@types/node 20系/node>=20.20.0のままで、konoka内のNodeサポートポリシーが不揃いです。意図的でなければ揃えることを検討してください。
軽微なスタイル (NOTE)
plugins/pr/src/bin/sync-and-push.tsのcatch節でprocess.exitCode = 1設定後のreturnは、戻り値型がPromise<void>のため省略可能です。他のCLIエントリ(prepare-editmsg.ts等)とtry/catchパターンも揃っていないため、共通のrunCli(main)ヘルパに集約すると意図が伝わりやすくなります。
There was a problem hiding this comment.
前回COMMENTレビュー以降の増分は2コミット(Node.js要件を>=22.19に統一, 不要なreturn削除)で、前回の主要指摘であったREADMEとengines.nodeのNodeバージョン不整合は解消されています。sync-and-push.tsのreturn削除も意図通りで問題ありません。
増分とPR全体を見直して新たに気付いた点を2つ挙げます。いずれも軽微で、本PR内での対応かフォローアップかは任せます。
package-lock.jsonのengines.nodeがまだ>=22.0.0のままで、package.jsonの>=22.19と乖離しています。npm install(またはnpm i --package-lock-only)で再生成して揃えてください。commit/prの両方に同じ問題があります。- 新しい
hooks/buildがnpm ci/npm run buildを要求するためnpmが事実上の実行要件になっていますが、commit/prのREADMEにはNode.jsしか書かれていません。kyoseiのREADMEと同様にnpmを要件に加え、自動ビルドの挙動を一文添えるとユーザの混乱が減ります。
コード品質・セキュリティ・性能・テスト観点では追加の指摘はありません。
- Node.jsのバージョン指定を ">=22.19" から ">=22.19.0" へ統一 - plugins/commit, plugins/pr, plugins/kyosei のREADMEとpackageファイルを修正 - バージョン表記の明確化により要件の誤解を防止
There was a problem hiding this comment.
前回COMMENTレビュー以降の増分は4コミットで、いずれも前回指摘へのフォローアップです。
plugins/commit/README.md/plugins/pr/README.mdにnpmの要件を追記package.jsonのengines.nodeを>=22.19から>=22.19.0に明示化し、package-lock.jsonも再生成して整合flake.nixのpkgs.nodejs_24をinherit (pkgs) nodejsへ変更し、nixpkgs stable (現在v22系) に揃え、@types/node22.19.19との整合性も改善
コード品質・依存・パフォーマンス・テスト・セキュリティ・ドキュメント各観点で追加の指摘はありません。前回スレッド (全て解決済み) で言及されていたkyoseiとのNodeバージョン方針差や共通化の提案はncaq/konokaの方針として明示的にcloseされており、本PR内での対応は不要です。
1 participant
bin/*.tsをNode.jsが直接実行していた構成から、viteによるバンドル方式に移行します。
外部ライブラリを追加した時の依存解決をビルド時へ寄せて挙動を明示化します。
主な変更内容は以下です。
bin/からsrc/bin/に移動し、viteでdist/bin/*.jsへバンドルbin/には薄いbashラッパーを配置し、readlink -fで自身の位置を解決してdist/bin/*.jsをexec nodessr.noExternal: trueで全依存をバンドルに含めるhooks/hooks.jsonとhooks/buildでSessionStartとPreToolUseの両方で自動ビルドtsconfig.jsonをbundler/preserveに切り替え、importの.ts拡張子を全削除editor.tsはLinuxの/usr/bin/editorとの衝突を避けてkonoka-editorにrenamepackage.jsonのbinフィールドは廃止(Claude Codeがbin/を自動PATH追加するため不要)engines.nodeをnixpkgs stableに合わせて>=22.0.0に統一kyoseiは
Bash(node:*)を広く許可してnode起動コマンドをSKILL.md本文に書きますが、今回はbashラッパーを噛ませることで個別の
Bash(<name>:*)許可を維持し、権限粒度を絞ったまま移行しています。