feat(plugin): requires.devbase をインストール時に検証する - #112
Conversation
plugin.yml の requires.devbase は値を読むだけで比較しておらず、宣言だけの 状態だった。project.yml 形式 (devbase 3.0.0 以降) の Plugin を 2.x へ インストールできてしまい、devbase up の段階で初めて失敗する。 devbase plugin install の経路 (repos 経由 / --link) で検証し、満たさない 場合は中止する。検証は既存インストールに触れる前に行う。後から落とすと、 入れ替えのために消した既存プラグインが戻らないまま失敗するため。 解釈できない書式や版数はエラーにせず警告に留める。独自記法を書いた Plugin を インストール不能にするより実害が小さく、依存が本当に足りなければ後段で失敗する。 検証側の判断が誤ったときのために DEVBASE_IGNORE_PLUGIN_REQUIRES=1 も用意した。 あわせて PLAN32 の plan にリリース後の対応と残作業を追記した。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | codex | REQUEST_CHANGES
requires.devbase の版数比較で、サポート外の要素を無視せず誤判定を防いでください。
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | gemini | REQUEST_CHANGES
総評・PR横断の指摘事項:
devbase plugin update時の考慮漏れ: 本 PR はplugin install時のチェックを追加していますが、devbase plugin updateコマンドで git pull を行った際にもプラグインが要求する devbase の版が上がり、互換性がなくなる可能性があります。lib/devbase/plugin/updater.pyの_update_repo_pluginsでも要件チェックを呼び出し、要件を満たさなくなったプラグインがある場合は警告やエラーを表示して、ユーザーに devbase 本体の更新を促す処理の追加を検討してください(事後報告であっても問題に気づけることが重要です)。
インストールをクラッシュさせる可能性がある AttributeError の指摘を含んでいるため、修正をお願いします。
cross-review round 1 の指摘対応。 - 版数の 4 要素目以降を [:3] で黙って切り捨てていたため、要件 >=3.0.0.1 を 現在版 3.0.0 が満たすと誤判定していた。切り捨てをやめ、比較時に短い方を 0 で 埋めて桁を揃える。">=3.0" と 3.0.0 が等しいことは変わらない。 - plugin.yml に `devbase: 3.0` とクォート無しで書くと YAML が float にするため、 .strip() が AttributeError になりインストールがクラッシュしていた。 load_plugin_info で str へ寄せ、検証側でも型を信用せず str() を通す。 - DEVBASE_IGNORE_PLUGIN_REQUIRES=FALSE / No のような書き方で、無効化のつもりが 無い値なのに検証が黙って飛んでいた。比較を小文字化して揃える。 - devbase plugin update は git pull で requires.devbase が上がっても素通しで、 devbase up まで気づけなかった。更新はもう済んでいて中止できないため、 warn_unmet_devbase_requirement で警告だけ出す。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVfeqnhZx7meRPUQAyBTta
🔧 /ndf:fix サマリ (round 1)対応件数: critical=1 / major=1 / minor=1 / review-body 指摘=1 (合計 4 件) 対応内容
#4 の設計判断
インストール経路は従来どおり 既存に触れる前に検証してエラーで中止 します(非対称なのは意図的です)。 追加テスト (+19 件)
ドキュメント ( @takemi-ohama 再レビューをお願いします。 |
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | codex | REQUEST_CHANGES
数値として解釈された requires.devbase が元の版表記を失い、互換性を誤判定するケースを修正してください。
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | gemini | APPROVE
Round 1 の指摘事項(型のゆれ、バージョンの切り捨て比較、検証スキップの環境変数の判定など)がすべて解消され、テストで保護されていることを確認しました。新たな修正提案はありません。
`devbase: 3.10` とクォート無しで書くと YAML は float の 3.1 として読む。 str へ寄せると "3.1" になり元の 3.10 へ戻せないため、現在版 3.1 が ">=3.10" を満たすと誤判定していた。 非文字列は復元不能として requires_devbase を None にし、クォートを促す 警告を出す。誤った版で比較するより、検証しない方が安全。requirements 側 にも同じ防御を残し、直接構築された PluginInfo でも落ちないようにした。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 3 | codex | REQUEST_CHANGES
update 時の plugin 分割移行で、互換性検証より先に既存登録を削除しないよう修正が必要です。
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 3 | gemini | APPROVE
実装は十分堅牢であり、エラー系やエッジケース(YAML起因の型ゆれ、不正な版数指定等)への防衛的対処・テストも網羅されています。追加の修正アクションはありません。
プラグイン分割の移行 (_migrate_removed_plugin) は旧登録を削除してから 移行先を登録する。移行先の requires.devbase が満たせないときに例外へ すると、移行先が登録されないまま旧登録も失われる。 git pull は既に済んでおり、ここで止めても registry と作業ツリーの整合は 取れない。_register_repo_plugin に enforce_requirements を足し、この経路 だけ警告に切り替える。install 経路は従来どおり中止する。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 4 | codex | APPROVE
修正必須の指摘はありません。
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 4 | gemini | APPROVE
実装を確認しました。PEP 440 の部分集合による比較ロジック、誤った型・未知の記法に遭遇した際のフォールバック(警告に留めて中断しない)、および更新時(update)の警告化など、既存の運用を壊さないための防御的な対応が徹底されており、非常に安定した設計だと思います。
テストケースも境界値や無効値、環境変数による迂回手段まで網羅されており、懸念点はありません。このままマージして問題ありません。
Summary
plugin.ymlのrequires.devbaseをdevbase plugin installの時点で検証し、要件を満たさない Plugin のインストールを中止します。これまで
requires.devbaseは値を読むだけで比較しておらず、宣言だけの状態でした。そのためproject.yml形式(devbase 3.0.0 以降)の Plugin を 2.x へインストールでき、devbase upの段階で初めて失敗していました。設計上の判断
--link経路は入れ替えのため既存ディレクトリを消してから登録するので、後から落とすと消した既存 Plugin が戻らないまま失敗します"^3.0.0"のような独自記法を書いた Plugin をインストール不能にするより実害が小さく、依存が本当に足りなければ後段で失敗しますDEVBASE_IGNORE_PLUGIN_REQUIRES=1で検証を無効化できます。検証側の判断が誤ったときに作業が詰まらないための逃げ道です>=/<=/>/</==/!=、カンマ区切りは AND、演算子省略は==)。比較は数値で行うため10.0.0 >= 3.0.0が正しく判定されます変更
lib/devbase/plugin/requirements.pyinstaller.pyの 2 つのインストール経路(repos 経由 /--link)へ組み込みdocs/plugin-dev/plugin-yml-reference.mdに書式一覧と検証の挙動を追記CHANGELOG.md、issues/PLAN32_multi-repo-project.md(リリース後の対応と残作業)を更新Test plan
uv run pytest tests/plugin/test_requirements.py— 31 件(満たす/満たさない各書式、要件なし、解釈不能な書式・版数の警告続行、環境変数での無効化、既定で動作中の devbase 版を見ること、インストール経路の中止、中止時に既存 Plugin が壊れないこと)uv run pytest— 全 1408 件 green