Skip to content

fix: コードレビュー指摘の修正(並行性・前方互換・堅牢性)#327

Merged
kkyosuke merged 1 commit into
mainfrom
review
Jun 24, 2026
Merged

fix: コードレビュー指摘の修正(並行性・前方互換・堅牢性)#327
kkyosuke merged 1 commit into
mainfrom
review

Conversation

@kkyosuke

Copy link
Copy Markdown
Owner

目的

最新 main のコードを複数観点(セキュリティ / 並行性・正確性 / アーキテクチャ / TUI / 永続化)でレビューし、検出した不具合をまとめて修正する。

変更内容

🔴 HIGH

  • Storage(workspaces.json) にプロセス間ロックを追加 (storage.rs / usecase/workspace.rs)
    add/remove/touch は read-modify-write なのにロックが無く、複数 usagi プロセス(複数 TUI+各セッションの usagi mcp)が同時に書くと last-writer-wins で登録や touch を取りこぼした。WorkspaceStore と同形の lock()(flock) を追加し、load→変更→save 全体を直列化。

🟠 MEDIUM

  • session create/remove のロック順を是正 (usecase/session/mod.rs / reconcile.rs)
    reconcile が store ロックのにロック無しで走り、未記録の worktree を stray として force 削除する競合があった。reconcile_locked を分離し、create(reconcile→worktree 構築→record) と remove をひとつのロックで保護。
  • JSON enum 未知値の前方互換ギャップ (domain/serde_fallback.rs ほか)
    enum フィールドの未知値が settings.json/state.json 全体の読み込みを失敗させた。or_default で該当フィールドのみ既定値に縮退(AgentCli/Theme/SessionActionUi/Sidebar/BranchSource/BranchStatus)。frontmatter の「未知キー無視」と挙動を揃えた。
  • PTY リサイズのロック順 (infrastructure/pty.rs)
    master.resize をロック外で先行実行していたため reflow 出力が旧サイズのグリッドで解釈され得た。parser ロック内で resize と set_size を原子化。
  • マウスホイールのペイン判定 (tui/home/terminal_pane.rs)
    列だけで判定していたためペインの上下でも誤スクロールした。pane_cell による両軸判定に統一。

🟡 LOW

  • MemoryStore::write_locked のパス検証 を read/remove と対称化(多層防御)。
  • TUI の tasks/sessions_refresh のロックを poison 時にクラッシュさせず復帰する方針(terminal_pool と同一)に統一。

📝 ドキュメント

  • domain の外部クレート依存(永続化の serde・時刻の chrono)を実態に合わせ規約を明確化。
  • モジュール表に agent_feature.rs / serde_fallback.rs / cli feature を追記。
  • state.json/settings.json の前方互換と workspaces.json のロックを data/ に記載。

検討の上で見送り(理由を明記)

  • レビューで挙がった SystemRunner/OllamaBackend の infrastructure への移設は見送り。CommandRunner トレイトは usecase にあり、impl だけを infrastructure に移すと infrastructure → usecase の依存逆流になる(本プロジェクトは usecase → infrastructure)。正しく直すにはトレイトを domain port へ抽出する大きめのリファクタが必要で、doctor 専用ユーティリティに対しては過剰。別 issue 化が妥当と判断。

テスト・確認方法

  • cargo fmt / cargo clippy --all-targets -- -D warnings 通過。
  • cargo test: 1676 passed, 0 failed。
  • カバレッジ(CI と同条件): Lines 100% / Functions 100% を維持(scripts/coverage.sh の閾値で EXIT=0)。
  • 追加テスト: Storage::lock、未知 enum の縮退(settings/state)、write のパス検証、ロック poison 復帰(tasks/sessions_refresh)。

🤖 Generated with Claude Code

多観点レビューで検出した不具合を修正する。

- Storage(workspaces.json) にプロセス間ロックを追加し、add/remove/touch の
  read-modify-write を直列化(更新の取りこぼし=lost update を防止)。
- session create/remove のロック順を是正。reconcile を store ロック内で実行する
  reconcile_locked に分離し、create→build→record をひとつのロックで保護して、
  並行する create がまだ未記録の worktree を stray として誤削除する競合を防ぐ。
- JSON enum の未知値が settings.json / state.json 全体の読み込みを失敗させる
  前方互換ギャップを修正。domain::serde_fallback::or_default で該当フィールドを
  既定値に縮退させる(AgentCli/Theme/SessionActionUi/Sidebar/BranchSource/
  BranchStatus)。frontmatter 側の「未知キーは無視」と挙動を揃える。
- PTY リサイズの順序を修正。parser ロック内で master.resize と set_size を
  原子的に行い、reflow 出力が旧サイズのグリッドで解釈される一過性の表示崩れを防ぐ。
- 埋め込みターミナルのマウスホイール判定を pane_cell に統一し、ペインの上下
  (タブ列・最終行より下)でも誤スクロールしていた問題を修正。
- MemoryStore::write_locked にも単一コンポーネント検証を適用し、read/remove と
  防御を対称化(パストラバーサルの多層防御)。
- TUI の tasks/sessions_refresh のロックを poison 時にクラッシュさせず復帰する
  方針(terminal_pool と同一)に統一。
- ドキュメント整合: domain の外部クレート依存(serde/chrono)を実態に合わせて
  規約を明確化、モジュール表に agent_feature.rs / serde_fallback.rs / cli feature を
  追記、state.json/settings.json の前方互換・workspaces.json のロックを記載。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📊 Test Coverage

🚀 いまのカバレッジ (Lines): 100.00% — この調子でいこう!

🎉✨ パーフェクト!全ファイル Lines カバレッジ 100% を達成しました 🏆🐰

@kkyosuke kkyosuke merged commit 37d31c4 into main Jun 24, 2026
4 checks passed
@kkyosuke kkyosuke deleted the review branch June 24, 2026 22:56
@kkyosuke kkyosuke mentioned this pull request Jun 25, 2026
kkyosuke added a commit that referenced this pull request Jun 25, 2026
## 目的

`1.2.0` → `1.2.1` のリリース。`Cargo.toml` の version を上げ、マージ時に
`auto-release.yml` がタグ `v1.2.1` とリリースを自動生成する。

## 変更内容

- `Cargo.toml` / `Cargo.lock` の version を `1.2.0` → `1.2.1` に更新。

### このリリースに含まれる変更(v1.2.0 以降)

- fix: コードレビュー指摘の修正(堅牢性・前方互換・セキュリティ) (#333)
- feat(tui): 在席で agent を指定して起動できるようにする (#334)
- refactor(session): 表示ラベル解決を presentation へ寄せ、new の Mode キーを統合する (#332)
- refactor(infra,usecase): version envelope と設定セッターのボイラープレートを削減する (#330)
- refactor(mcp): issue/memory の filter Args を flatten で一本化する (#329)
- refactor(agent): program 名を AgentCli::command に一本化する (#328)
- feat(tui): コマンド引数(サブコマンド・オプション)も Tab 補完する (#331)
- refactor(tui): widgets から rabbit アセットを分離する (#323)
- fix: コードレビュー指摘の修正(並行性・前方互換・堅牢性) (#327)
- feat(error-log): 静かに握りつぶされる本物の失敗をログに記録 (#325)
- fix: アップデート告知と更新後メッセージの文言を変更 (#324)

## テスト・確認方法

- version のみの変更。`release-build-check.yml` が 4 プラットフォームのリリースビルドを検証する。
- マージ後、`auto-release.yml` → `release.yml` でタグ `v1.2.1` と GitHub Release
が自動生成される。

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant