From 4a96b872d95c5ae35488d309a7c2fc23c16ac99f Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 22 Mar 2026 18:09:16 +0900 Subject: [PATCH 1/2] =?UTF-8?q?feat(config):=20=E3=83=81=E3=83=BC=E3=83=A0?= =?UTF-8?q?=E5=85=B1=E6=9C=89=E8=A8=AD=E5=AE=9A=E3=83=95=E3=82=A1=E3=82=A4?= =?UTF-8?q?=E3=83=AB=E5=AE=9F=E8=A3=85=20(#76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit チームで共有可能な commandindex.toml をリポジトリルートに配置し、 インデックス設定やEmbeddingプロバイダー設定をチーム全体で統一できるようにする。 主な変更: - config モジュール新規作成（src/config/mod.rs） - 設定ファイル優先順位: env > config.local.toml > commandindex.toml > 旧config.toml > デフォルト - フィールドレベルマージによる設定統合 - config show / config path CLIサブコマンド追加 - 既存 embedding::Config を config::load_config() に移行 - 秘匿値（api_key）のマスク表示とチーム設定での api_key 禁止バリデーション - 旧 .commandindex/config.toml の deprecated fallback 対応 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../issue-76-team-config-design-policy.md | 569 ++++++++++ .../issue-review/hypothesis-verification.md | 43 + .../issue/76/issue-review/original-issue.json | 1 + .../issue-review/stage1-review-context.json | 75 ++ .../76/issue-review/stage2-apply-result.json | 18 + .../issue-review/stage3-review-context.json | 75 ++ .../76/issue-review/stage4-apply-result.json | 17 + .../issue-review/stage5-review-context.json | 46 + .../76/issue-review/stage6-apply-result.json | 14 + .../issue-review/stage7-review-context.json | 64 ++ .../76/issue-review/stage8-apply-result.json | 15 + .../issue/76/issue-review/summary-report.md | 63 ++ .../stage1-4-apply-result.json | 27 + .../stage1-review-context.json | 20 + .../stage2-review-context.json | 23 + .../stage3-review-context.json | 23 + .../stage4-review-context.json | 21 + .../summary-report.md | 46 + .../pm-auto-dev/iteration-1/tdd-context.json | 19 + .../pm-auto-dev/iteration-1/tdd-result.json | 21 + dev-reports/issue/76/work-plan.md | 201 ++++ src/cli/clean.rs | 10 +- src/cli/config.rs | 66 ++ src/cli/embed.rs | 19 +- src/cli/index.rs | 21 +- src/cli/mod.rs | 1 + src/cli/search.rs | 64 +- src/config/mod.rs | 969 ++++++++++++++++++ src/embedding/mod.rs | 125 +-- src/embedding/openai.rs | 24 +- src/lib.rs | 1 + src/main.rs | 75 +- src/rerank/mod.rs | 32 +- tests/cli_args.rs | 109 +- tests/e2e_embedding.rs | 10 +- tests/e2e_semantic_hybrid.rs | 9 +- 36 files changed, 2742 insertions(+), 194 deletions(-) create mode 100644 dev-reports/design/issue-76-team-config-design-policy.md create mode 100644 dev-reports/issue/76/issue-review/hypothesis-verification.md create mode 100644 dev-reports/issue/76/issue-review/original-issue.json create mode 100644 dev-reports/issue/76/issue-review/stage1-review-context.json create mode 100644 dev-reports/issue/76/issue-review/stage2-apply-result.json create mode 100644 dev-reports/issue/76/issue-review/stage3-review-context.json create mode 100644 dev-reports/issue/76/issue-review/stage4-apply-result.json create mode 100644 dev-reports/issue/76/issue-review/stage5-review-context.json create mode 100644 dev-reports/issue/76/issue-review/stage6-apply-result.json create mode 100644 dev-reports/issue/76/issue-review/stage7-review-context.json create mode 100644 dev-reports/issue/76/issue-review/stage8-apply-result.json create mode 100644 dev-reports/issue/76/issue-review/summary-report.md create mode 100644 dev-reports/issue/76/multi-stage-design-review/stage1-4-apply-result.json create mode 100644 dev-reports/issue/76/multi-stage-design-review/stage1-review-context.json create mode 100644 dev-reports/issue/76/multi-stage-design-review/stage2-review-context.json create mode 100644 dev-reports/issue/76/multi-stage-design-review/stage3-review-context.json create mode 100644 dev-reports/issue/76/multi-stage-design-review/stage4-review-context.json create mode 100644 dev-reports/issue/76/multi-stage-design-review/summary-report.md create mode 100644 dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-context.json create mode 100644 dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-result.json create mode 100644 dev-reports/issue/76/work-plan.md create mode 100644 src/cli/config.rs create mode 100644 src/config/mod.rs diff --git a/dev-reports/design/issue-76-team-config-design-policy.md b/dev-reports/design/issue-76-team-config-design-policy.md new file mode 100644 index 0000000..5d5aebc --- /dev/null +++ b/dev-reports/design/issue-76-team-config-design-policy.md @@ -0,0 +1,569 @@ +# 設計方針書 - Issue #76: チーム共有設定ファイル（config.toml） + +## 1. 概要 + +| 項目 | 内容 | +|------|------| +| Issue | #76 [Feature] チーム共有設定ファイル（config.toml） | +| 種別 | 新機能 | +| 影響範囲 | 中規模（config新設 + 既存6箇所の移行 + CLI追加） | +| 作成日 | 2026-03-22 | +| レビュー反映 | Stage 1-4 完了（設計原則・整合性・影響分析・セキュリティ） | + +## 2. システムアーキテクチャ上の位置づけ + +### 現状のアーキテクチャ + +``` +┌──────────┐ +│ main.rs │ CLI エントリポイント (clap) +└────┬─────┘ + │ +┌────┴─────────────────────────────────────────┐ +│ cli/ │ +│ search.rs embed.rs index.rs clean.rs ... │ +└────┬──────────┬──────────┬───────────────────┘ + │ │ │ +┌────┴────┐ ┌──┴───┐ ┌───┴────┐ +│embedding│ │rerank│ │indexer │ ... +│ Config │ │Config│ │ │ +└─────────┘ └──────┘ └────────┘ +``` + +**問題点**: 設定読み込みが `embedding::Config` に集約されており、embedding/rerank 以外の設定（search, index）を扱えない。 + +### 新アーキテクチャ + +``` +┌──────────┐ +│ main.rs │ CLI エントリポイント (clap) +└────┬─────┘ + │ +┌────┴─────────────────────────────────────────────┐ +│ cli/ │ +│ search.rs embed.rs index.rs clean.rs config.rs│ +└────┬──────────┬──────────┬───────────────────────┘ + │ │ │ + └──────────┴──────────┘ + │ + ┌──────────┴──────────┐ + │ config/mod.rs │ ← 新規: 唯一の設定読込入口 + │ load_config() │ ← ローダー関数（SRP: データと I/O を分離） + │ AppConfig (データ) │ + └──┬───┬───┬──────────┘ + │ │ │ + ┌──┘ │ └──────┐ + │ │ │ +┌────┴────┐ ┌──┴───┐ ┌───┴────┐ +│embedding│ │rerank│ │indexer │ +│ Config │ │Config│ │ │ +│ (型のみ)│ │(型のみ)│ │ +└─────────┘ └──────┘ └────────┘ +``` + +**設計原則**: config モジュールを唯一の設定読込入口とし、各 CLI コマンドは `load_config()` を1回だけ呼び出して `&AppConfig` を各関数に引き回す。 + +## 3. レイヤー構成と責務 + +| レイヤー | モジュール | 変更種別 | 責務 | +|---------|-----------|---------|------| +| **Config（新規）** | `src/config/mod.rs` | 新規作成 | 設定ファイルの読み込み・マージ・バリデーション | +| **CLI** | `src/main.rs` | 変更 | Commands enum に Config サブコマンド追加、検索引数を Option 化 | +| **CLI** | `src/cli/config.rs` | 新規作成 | config show / config path サブコマンド実装 | +| **CLI** | `src/cli/mod.rs` | 変更 | `pub mod config;` 追加 | +| **CLI** | `src/cli/search.rs` | 変更 | Config::load() → load_config() への移行（4箇所: L130, L291, L424, L650）、関数シグネチャに &AppConfig 追加 | +| **CLI** | `src/cli/embed.rs` | 変更 | Config::load() → load_config() への移行（1箇所: L110） | +| **CLI** | `src/cli/index.rs` | 変更 | Config::load() → load_config() への移行（1箇所: L795） | +| **CLI** | `src/cli/clean.rs` | 変更 | 保持対象ファイル名の更新（embeddings.db, config.toml, config.local.toml） | +| **Embedding** | `src/embedding/mod.rs` | 変更 | Config 構造体と load() を削除、ProviderType に Serialize 追加 | +| **Embedding** | `src/embedding/openai.rs` | 変更 | OpenAiProvider に Custom Debug 実装（api_key マスク） | +| **Rerank** | `src/rerank/mod.rs` | 変更 | RerankConfig に Serialize 追加、Custom Debug 実装（api_key マスク） | +| **Lib** | `src/lib.rs` | 変更 | `pub mod config;` 追加 | + +## 4. 新規モジュール設計: `src/config/mod.rs` + +### 4.1 定数定義 + +```rust +/// チーム共有設定ファイル（リポジトリルート） +pub const TEAM_CONFIG_FILE: &str = "commandindex.toml"; +/// ローカル個人設定ファイル（.commandindex/ 配下） +pub const LOCAL_CONFIG_FILE: &str = "config.local.toml"; +/// 旧設定ファイル（deprecated fallback） +pub const LEGACY_CONFIG_FILE: &str = "config.toml"; +``` + +### 4.2 エラー型 + +**方針**: 既存プロジェクトのパターン（手動 Display + Error 実装）に合わせる。thiserror は導入しない。 + +```rust +#[derive(Debug)] +pub enum ConfigError { + ReadError { + path: PathBuf, + source: std::io::Error, + }, + ParseError { + path: PathBuf, + source: toml::de::Error, + }, + SerializeError(toml::ser::Error), + /// チーム共有設定に api_key が含まれている（セキュリティ違反） + SecretInTeamConfig { + path: PathBuf, + field: String, + }, +} + +impl std::fmt::Display for ConfigError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::ReadError { path, source } => + write!(f, "Failed to read config file '{}': {}", path.display(), source), + Self::ParseError { path, source } => + write!(f, "Failed to parse config file '{}': {}", path.display(), source), + Self::SerializeError(e) => + write!(f, "Failed to serialize config: {}", e), + Self::SecretInTeamConfig { path, field } => + write!(f, "Security: '{}' contains '{}'. API keys must be in config.local.toml or environment variables.", + path.display(), field), + } + } +} + +impl std::error::Error for ConfigError {} +``` + +**エラー型伝播**: 各 CLI モジュールのエラー型に `From` を実装する。 + +```rust +// 例: cli/search.rs +impl From for SearchError { + fn from(e: ConfigError) -> Self { + SearchError::Config(e.to_string()) + } +} +``` + +### 4.3 マージ用中間構造体（RawConfig） + +```rust +/// TOML ファイルから読み込む中間構造体（全フィールド Option） +#[derive(Debug, Default, Deserialize)] +pub struct RawConfig { + pub index: Option, + pub search: Option, + pub embedding: Option, + pub rerank: Option, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawSearchConfig { + pub default_limit: Option, + pub snippet_lines: Option, + pub snippet_chars: Option, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawIndexConfig { + pub languages: Option>, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawEmbeddingConfig { + pub provider: Option, + pub model: Option, + pub endpoint: Option, + pub api_key: Option, +} + +/// RerankConfig のマージ用中間構造体 +/// 注: provider フィールドは含まない（既存 RerankConfig に provider がなく、Ollama 固定） +#[derive(Debug, Default, Deserialize)] +pub struct RawRerankConfig { + pub model: Option, + pub top_candidates: Option, + pub endpoint: Option, + pub api_key: Option, + pub timeout_secs: Option, +} +``` + +**DRY 対策**: RawConfig と最終型のフィールド同期を保証するため、テストで全フィールドのラウンドトリップ検証を実装する。 + +### 4.4 最終設定構造体（AppConfig） + +**注**: AppConfig 自体には `Serialize` を付与しない（api_key 露出防止）。表示は `to_masked_view()` 経由のみ。 + +```rust +/// マージ済みの最終設定（Serialize なし: 秘匿値保護） +#[derive(Debug, Clone)] +pub struct AppConfig { + pub index: IndexConfig, + pub search: SearchConfig, + pub embedding: EmbeddingConfig, // embedding::EmbeddingConfig を再利用 + pub rerank: RerankConfig, // rerank::RerankConfig を再利用 + /// 読み込まれた設定ファイルのパス情報 + pub loaded_sources: Vec, +} + +#[derive(Debug, Clone)] +pub struct ConfigSource { + pub path: PathBuf, + pub kind: ConfigSourceKind, +} + +#[derive(Debug, Clone)] +pub enum ConfigSourceKind { + Team, // commandindex.toml + Local, // .commandindex/config.local.toml + Legacy, // .commandindex/config.toml (deprecated) +} + +#[derive(Debug, Clone, Serialize)] +pub struct IndexConfig { + pub languages: Vec, +} + +#[derive(Debug, Clone, Serialize)] +pub struct SearchConfig { + pub default_limit: usize, // デフォルト: 20 + pub snippet_lines: usize, // デフォルト: 2 + pub snippet_chars: usize, // デフォルト: 120 +} +``` + +### 4.5 ローダー関数（SRP: データと I/O を分離） + +```rust +/// 設定を読み込み、優先順位に従ってマージする（公開 API） +/// +/// 優先順位: 環境変数 > config.local.toml > commandindex.toml > 旧config.toml > デフォルト +/// +/// base_path の決定: +/// - --path を持つコマンド（index, update, embed, clean）: --path の値 +/// - --path を持たないコマンド（search, config）: カレントディレクトリ "." +pub fn load_config(base_path: &Path) -> Result { + let mut sources = Vec::new(); + let mut merged = RawConfig::default(); + + let legacy_path = base_path.join(".commandindex").join(LEGACY_CONFIG_FILE); + let team_path = base_path.join(TEAM_CONFIG_FILE); + let local_path = base_path.join(".commandindex").join(LOCAL_CONFIG_FILE); + + // 1. 旧設定ファイル（deprecated fallback） + if legacy_path.exists() { + if team_path.exists() { + eprintln!("Warning: {} is ignored because {} exists.", + legacy_path.display(), team_path.display()); + } else { + let raw = read_toml(&legacy_path)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { path: legacy_path, kind: ConfigSourceKind::Legacy }); + eprintln!("Warning: {} is deprecated. Please migrate to {}", + legacy_path.display(), team_path.display()); + } + } + + // 2. チーム共有設定（api_key バリデーション付き） + if team_path.exists() { + let raw = read_toml(&team_path)?; + validate_no_secrets(&team_path, &raw)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { path: team_path, kind: ConfigSourceKind::Team }); + } + + // 3. ローカル個人設定 + if local_path.exists() { + let raw = read_toml(&local_path)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { path: local_path, kind: ConfigSourceKind::Local }); + } + + // 4. RawConfig → AppConfig に変換（デフォルト値適用） + // 環境変数は EmbeddingConfig::resolve_api_key() に委譲 + Ok(resolve_config(merged, sources)) +} + +/// チーム共有設定に api_key が含まれていないことを検証 +fn validate_no_secrets(path: &Path, raw: &RawConfig) -> Result<(), ConfigError> { + if let Some(ref emb) = raw.embedding { + if emb.api_key.is_some() { + return Err(ConfigError::SecretInTeamConfig { + path: path.to_path_buf(), + field: "embedding.api_key".to_string(), + }); + } + } + if let Some(ref rer) = raw.rerank { + if rer.api_key.is_some() { + return Err(ConfigError::SecretInTeamConfig { + path: path.to_path_buf(), + field: "rerank.api_key".to_string(), + }); + } + } + Ok(()) +} + +/// フィールドレベルマージ: higher が優先 +fn merge_raw(base: RawConfig, higher: RawConfig) -> RawConfig { + // 各フィールドで higher が Some なら higher、None なら base を採用 +} + +fn read_toml(path: &Path) -> Result { + let content = std::fs::read_to_string(path) + .map_err(|e| ConfigError::ReadError { path: path.to_path_buf(), source: e })?; + toml::from_str(&content) + .map_err(|e| ConfigError::ParseError { path: path.to_path_buf(), source: e }) +} + +fn resolve_config(raw: RawConfig, sources: Vec) -> AppConfig { + // RawConfig の Option フィールドにデフォルト値を適用して AppConfig に変換 +} +``` + +### 4.6 config show 用 view model + +AppConfig からの変換のみ。`Serialize` は view model にのみ付与。 + +```rust +/// 秘匿値をマスクした表示用構造体 +#[derive(Serialize)] +pub struct AppConfigView { + pub index: IndexConfig, + pub search: SearchConfig, + pub embedding: EmbeddingConfigView, + pub rerank: RerankConfigView, +} + +#[derive(Serialize)] +pub struct EmbeddingConfigView { + pub provider: String, // ProviderType を文字列に変換 + pub model: String, + pub endpoint: String, + pub api_key: String, // "***" or "(not set)" +} + +#[derive(Serialize)] +pub struct RerankConfigView { + pub model: String, + pub top_candidates: usize, + pub endpoint: String, + pub api_key: String, // "***" or "(not set)" + pub timeout_secs: u64, +} + +impl AppConfig { + pub fn to_masked_view(&self) -> AppConfigView { + // api_key を "***" にマスクした view model を生成 + // ProviderType は to_string() で文字列化 + } +} +``` + +## 5. CLI設計 + +### 5.1 Commands enum 変更 + +```rust +// src/main.rs +#[derive(Subcommand)] +enum Commands { + // ... 既存コマンド ... + /// 設定の表示・管理 + Config { + #[command(subcommand)] + command: ConfigCommands, + }, +} + +#[derive(Subcommand)] +enum ConfigCommands { + /// 現在の有効設定を表示（秘匿値はマスク） + Show, + /// 読み込まれた設定ファイルのパスを表示 + Path, +} +``` + +### 5.2 検索引数の Option 化 + +```rust +// 変更前 +#[arg(long, default_value_t = 20)] +limit: usize, + +// 変更後 +#[arg(long, help = "Maximum number of results (default: from config or 20)")] +limit: Option, +``` + +**値解決フロー**: +```rust +let config = load_config(base_path)?; +let limit = cli_limit.unwrap_or(config.search.default_limit); +``` + +### 5.3 search.rs の AppConfig 引き回し + +```rust +// run() で1回だけロードし、各内部関数に &AppConfig を渡す +pub fn run(/* ... */) -> Result<(), SearchError> { + let config = load_config(&base_path)?; + // ... + try_hybrid_search(/* ... */, &config)?; + try_rerank(/* ... */, &config)?; +} + +fn try_hybrid_search(/* ... */, config: &AppConfig) -> Result<(), SearchError> { + // config.embedding, config.rerank を使用 +} + +fn try_rerank(/* ... */, config: &AppConfig) -> Result<(), SearchError> { + // config.rerank を使用 +} +``` + +## 6. 設定ファイル優先順位のフロー図 + +``` + ┌─────────────┐ + │ CLI引数 │ 明示指定時のみ + └──────┬──────┘ + │ (None なら次へ) + ┌──────┴──────┐ + │ 環境変数 │ COMMANDINDEX_OPENAI_API_KEY + └──────┬──────┘ + │ (None なら次へ) + ┌──────┴──────┐ + │config.local │ .commandindex/config.local.toml + └──────┬──────┘ + │ (None なら次へ) + ┌──────┴──────┐ + │commandindex │ commandindex.toml (チーム共有) + │ │ ※ api_key 禁止（バリデーション） + └──────┬──────┘ + │ (None なら次へ) + ┌──────┴──────┐ + │旧config.toml│ .commandindex/config.toml (deprecated) + └──────┬──────┘ + │ (None なら次へ) + ┌──────┴──────┐ + │デフォルト値 │ ハードコード + └─────────────┘ +``` + +## 7. 設計判断とトレードオフ + +### 判断1: RawConfig（中間構造体）パターン + +**選択**: 全フィールド `Option` の `RawConfig` でファイルを読み込み、マージ後に `AppConfig` に変換 +**理由**: フィールドレベルマージには「未指定」と「デフォルト値」を区別する必要がある +**トレードオフ**: 構造体が2重定義になるが、マージの正確性が保証される +**DRY 対策**: テストでフィールド同期のラウンドトリップ検証を実装 + +### 判断2: EmbeddingConfig/RerankConfig の配置 + +**選択**: 型定義は `embedding/mod.rs`, `rerank/mod.rs` に残す +**理由**: 循環依存を回避。各モジュールが自身の型を所有し、config モジュールがそれを参照 +**トレードオフ**: config モジュールが embedding/rerank に依存するが、逆方向の依存は発生しない + +### 判断3: deprecated fallback の維持 + +**選択**: 旧 `.commandindex/config.toml` を deprecated fallback として読み込み継続 +**理由**: breaking change を回避し、既存ユーザーの動作を維持 +**トレードオフ**: コード複雑性が増すが、移行期間中のユーザー体験を優先 + +### 判断4: config show の view model 分離 + +**選択**: `AppConfigView` を別途作成し、秘匿値をマスク。AppConfig 自体に Serialize は付与しない +**理由**: AppConfig に Serialize を付けると api_key が平文出力される潜在リスク +**トレードオフ**: View 構造体が増えるが、セキュリティが保証される + +### 判断5: CLI引数の Option 化 + +**選択**: `--limit` 等を `Option` に変更 +**理由**: clap の `default_value_t` では「ユーザーが明示指定したか」を判別できない +**対策**: help テキストにデフォルト値を明示して UX 劣化を最小化 + +### 判断6: ローダー関数の分離（SRP） + +**選択**: `load_config()` を公開関数として分離し、`AppConfig` は純粋なデータ構造に +**理由**: SRP/OCP準拠。テスタビリティ向上（ファイルシステムに依存しないテストが可能） + +### 判断7: チーム共有設定での api_key 禁止 + +**選択**: `commandindex.toml`（Git 管理対象）に api_key が含まれる場合はエラーとする +**理由**: Git にコミットされる設定ファイルに秘匿情報を含めるのはセキュリティリスク +**代替**: api_key は `config.local.toml`（.gitignore 対象）または環境変数のみに許可 + +### 判断8: RawRerankConfig に provider フィールドを含めない + +**選択**: 既存 RerankConfig に provider フィールドがないため、Raw にも含めない +**理由**: 現在 Ollama 固定。YAGNI 原則に従い、不要なフィールドは追加しない + +## 8. 影響範囲マトリクス + +| ファイル | 変更内容 | リスク | +|---------|---------|--------| +| `src/config/mod.rs` | 新規作成 | 低（新規） | +| `src/cli/config.rs` | 新規作成 | 低（新規） | +| `src/cli/mod.rs` | `pub mod config;` 追加 | 低 | +| `src/lib.rs` | `pub mod config;` 追加 | 低 | +| `src/main.rs` | Commands enum 追加、検索引数 Option 化 | 中（既存動作変更） | +| `src/embedding/mod.rs` | Config 削除、ProviderType に Serialize 追加 | 高（6箇所の呼び出し影響） | +| `src/embedding/openai.rs` | Custom Debug 実装（api_key マスク） | 低 | +| `src/rerank/mod.rs` | Serialize 追加、Custom Debug 実装（api_key マスク） | 低 | +| `src/cli/search.rs` | load_config() 1回呼出 + &AppConfig 引き回し（関数シグネチャ変更） | 中 | +| `src/cli/embed.rs` | Config::load → load_config()（1箇所: L110） | 低 | +| `src/cli/index.rs` | Config::load → load_config()（1箇所: L795） | 低 | +| `src/cli/clean.rs` | 保持対象: embeddings.db, config.toml, config.local.toml | 中 | +| `tests/e2e_embedding.rs` | 設定ファイルパス更新 + legacy fallback テスト追加 | 中 | +| `tests/e2e_semantic_hybrid.rs` | 設定ファイルパス更新 | 中 | +| `tests/cli_args.rs` | config show/path テスト追加、help 出力に "config" 含む検証 | 低 | + +## 9. セキュリティ設計 + +| 脅威 | 対策 | 優先度 | +|------|------|--------| +| チーム設定への API キー混入 | `validate_no_secrets()` でチーム設定の api_key を拒否 | **高** | +| config show での API キー平文表示 | view model 経由のマスク表示（`***`）。AppConfig に Serialize なし | 高 | +| RerankConfig Debug での api_key 露出 | Custom Debug impl でマスク（既存の EmbeddingConfig と同様） | 高 | +| OpenAiProvider Debug での api_key 露出 | Custom Debug impl でマスク | 高 | +| 設定ファイル経由の不正値注入 | TOML パースエラーで早期失敗 | 中 | +| パストラバーサル | base_path 基準のハードコードファイル名のみ使用 | 中 | +| 環境変数経由の秘匿情報漏洩 | resolve_api_key() の既存パターンを維持 | 低 | + +## 10. テスト戦略 + +### 単体テスト（config モジュール内） +- `merge_raw()`: フィールドレベルマージの正確性 +- `load_config()`: ファイル読み込み・優先順位・deprecated fallback +- `validate_no_secrets()`: チーム設定の api_key 拒否 +- `to_masked_view()`: 秘匿値マスクの正確性 +- `ConfigError`: エラー型の表示 +- **DRY 検証**: RawConfig ↔ AppConfig のフィールド同期テスト + +### 統合テスト +- E2E 3系統: 新設定のみ / ローカル上書き / レガシーfallback +- CLI引数優先順位: 未指定（ハードコードデフォルト） / 設定あり（設定値） / CLI明示（CLI優先） +- config show / config path サブコマンド +- clean --keep-embeddings 回帰テスト（config.local.toml 保持確認） +- 各コマンドのベースパス別設定検出テスト（--path 有無） +- cli_args.rs: help 出力に "config" サブコマンドが含まれること + +### テスト移行計画 +- e2e_embedding.rs: `.commandindex/config.toml` → `commandindex.toml`（リポジトリルート）に変更 +- e2e_semantic_hybrid.rs: `create_test_config()` を `commandindex.toml` に変更 +- legacy fallback テスト: 旧 `.commandindex/config.toml` のみ存在するケースを新規追加 + +## 11. 品質基準 + +| チェック項目 | コマンド | 基準 | +|-------------|----------|------| +| ビルド | `cargo build` | エラー0件 | +| Clippy | `cargo clippy --all-targets -- -D warnings` | 警告0件 | +| テスト | `cargo test --all` | 全テストパス | +| フォーマット | `cargo fmt --all -- --check` | 差分なし | diff --git a/dev-reports/issue/76/issue-review/hypothesis-verification.md b/dev-reports/issue/76/issue-review/hypothesis-verification.md new file mode 100644 index 0000000..4472389 --- /dev/null +++ b/dev-reports/issue/76/issue-review/hypothesis-verification.md @@ -0,0 +1,43 @@ +# 仮説検証レポート - Issue #76 + +## 検証結果サマリー + +| 仮説 | 判定 | 詳細 | +|------|------|------| +| `config` モジュールを `src/config/mod.rs` に新規作成 | **Confirmed** | 現在configモジュールは存在しない。新規作成が必要 | +| `toml` crateで設定ファイルをパース | **Confirmed** | `toml = "0.8"` が既にCargo.tomlに含まれている | +| 既存の `embedding` モジュールの設定をconfig経由に移行 | **Partially Confirmed** | 既に `embedding/mod.rs` に `Config` 構造体が存在し `.commandindex/config.toml` を読んでいる。これを新configモジュールに移行する必要がある | + +## 詳細検証 + +### 1. configモジュールの現状 +- `src/config/mod.rs` は存在しない +- 現在のモジュール: `cli/`, `embedding/`, `indexer/`, `output/`, `parser/`, `rerank/`, `search/` +- 設定読み込みは `embedding/mod.rs` の `Config::load()` に集約されている + +### 2. toml crateの状況 +- `Cargo.toml` line 27: `toml = "0.8"` として既に依存に含まれている +- 現在は `embedding/mod.rs` で使用されている + +### 3. 既存の設定基盤 +- **Config構造体**: `embedding/mod.rs` に `Config` (= `EmbeddingConfig` + `RerankConfig`) +- **読み込み元**: `.commandindex/config.toml` のみ（ハードコード） +- **環境変数**: `COMMANDINDEX_OPENAI_API_KEY` のみ対応 +- **優先順位**: 環境変数 > config.toml > デフォルト値（部分的に実装済み） + +### 4. CLIサブコマンドの現状 +- 現在のサブコマンド: `index`, `search`, `update`, `status`, `clean`, `context`, `embed` +- `config` サブコマンドは未実装 + +### 5. 追加発見事項 +- `.cmindexignore` ファイルによるignoreパターンのサポートが既に存在（`src/parser/ignore.rs`） +- `RerankConfig` も `.commandindex/config.toml` から読み込まれている +- `.commandindex/` ディレクトリは tantivy index, SQLite DB, config, manifest, state を格納 + +## Issue記載内容への影響 + +### 修正推奨事項 +1. **設定ファイル階層**: Issue提案の4層構造は妥当だが、既存の `.commandindex/config.toml` との後方互換性を明記すべき +2. **既存Config移行**: `embedding::Config` から新 `config::Config` への移行パスを明確にすべき +3. **RerankConfig**: Issue本文に `[rerank]` セクションが含まれているが、既存の `RerankConfig` フィールド（`top_candidates`, `timeout_secs`）との整合性を確認すべき +4. **ignoreパターン**: `.cmindexignore` との関係性を検討すべき（将来的にconfigに統合するか） diff --git a/dev-reports/issue/76/issue-review/original-issue.json b/dev-reports/issue/76/issue-review/original-issue.json new file mode 100644 index 0000000..18e4aaf --- /dev/null +++ b/dev-reports/issue/76/issue-review/original-issue.json @@ -0,0 +1 @@ +{"body":"## 概要\n\nチームで共有可能な設定ファイル `commandindex.toml` をリポジトリルートに配置し、インデックス設定やEmbeddingプロバイダー設定をチーム全体で統一できるようにする。\n\n## 背景・動機\n\n現在の設定は `.commandindex/` 内に閉じており、`.gitignore` で除外されるためチーム共有ができない。チームで同じ設定（除外パターン、Embeddingモデル、検索デフォルト等）を使いたい場合に、各メンバーが個別に設定する必要がある。\n\n## 提案する解決策\n\n### 設定ファイルの優先順位\n\n```\n1. 環境変数 (COMMANDINDEX_*) ← 最優先（個人オーバーライド）\n2. .commandindex/config.local.toml ← ローカル個人設定（.gitignore対象）\n3. commandindex.toml ← リポジトリルート（チーム共有、Git管理）\n4. デフォルト値 ← フォールバック\n```\n\n### 設定ファイルフォーマット\n\n```toml\n# commandindex.toml（チーム共有）\n\n[index]\n# 対象言語の優先設定\nlanguages = [\"markdown\", \"typescript\", \"python\"]\n\n[search]\n# デフォルトの検索結果件数\ndefault_limit = 20\n# デフォルトのスニペット行数\nsnippet_lines = 2\nsnippet_chars = 120\n\n[embedding]\n# チーム統一のEmbeddingプロバイダー\nprovider = \"ollama\"\nmodel = \"nomic-embed-text\"\nendpoint = \"http://localhost:11434\"\n\n[rerank]\nprovider = \"ollama\"\nmodel = \"bge-reranker-v2-m3\"\n```\n\n### CLI連携\n\n```bash\n# 現在の有効設定を表示\ncommandindex config show\n\n# 設定ファイルの場所を表示\ncommandindex config path\n```\n\n### 実装方針\n\n- `config` モジュールを新規作成（`src/config/mod.rs`）\n- `toml` crateで設定ファイルをパース\n- 既存の `embedding` モジュールの設定をconfig経由に移行\n- `Config` 構造体でマージロジックを実装\n\n## 受け入れ基準\n\n- [ ] `commandindex.toml` をリポジトリルートに配置してチーム共有設定ができる\n- [ ] `.commandindex/config.local.toml` で個人オーバーライドができる\n- [ ] 環境変数で最優先オーバーライドができる\n- [ ] `commandindex config show` で有効設定を表示できる\n- [ ] 既存のEmbedding設定がconfig経由で動作する\n- [ ] 設定ファイルが存在しない場合はデフォルト値で動作する（後方互換）\n- [ ] cargo test / clippy / fmt 全パス\n\n## 依存 Issue\n\n- なし（Phase 5完了が前提）","title":"[Feature] チーム共有設定ファイル（config.toml）"} diff --git a/dev-reports/issue/76/issue-review/stage1-review-context.json b/dev-reports/issue/76/issue-review/stage1-review-context.json new file mode 100644 index 0000000..04f91ba --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage1-review-context.json @@ -0,0 +1,75 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "Config構造体の命名・配置場所の衝突", + "description": "現在 embedding::Config が src/embedding/mod.rs に存在し、embedding と rerank の両セクションを管理している。Issueでは src/config/mod.rs に新しい Config モジュールを作成するとしているが、既存の embedding::Config との関係（移行手順、互換性、既存の6箇所の Config::load 呼び出しの書き換え）が明記されていない。既存コードでは crate::embedding::Config::load() が cli/search.rs(4箇所)、cli/embed.rs(1箇所)、cli/index.rs(1箇所) で使用されている。", + "suggestion": "Issueの実装方針に「既存 embedding::Config を config モジュールへ移行し、embedding::Config は削除する」「全呼び出し箇所(cli/search.rs, cli/embed.rs, cli/index.rs)のインポートパスを更新する」といった移行ステップを明記すべき。" + }, + { + "id": "M2", + "title": "マージロジックの仕様が不明確", + "description": "4段階の優先順位（環境変数 > config.local.toml > commandindex.toml > デフォルト）を提案しているが、マージの粒度が未定義。フィールドレベルのマージかセクションレベルのマージかを明記する必要がある。", + "suggestion": "「フィールドレベルでマージする（Option::None のフィールドは下位優先度の値を維持）」と明記し、マージの具体例を1つ追加すべき。" + }, + { + "id": "M3", + "title": "環境変数オーバーライドの範囲が不明確", + "description": "現在の環境変数は COMMANDINDEX_OPENAI_API_KEY のみ。Issueでは COMMANDINDEX_* と記載しているが、どの設定項目を環境変数で上書き可能にするのかが未定義。全項目対応とするとスコープが大きく膨らむ。", + "suggestion": "Phase 1として既存の COMMANDINDEX_OPENAI_API_KEY のみをサポートし、環境変数の網羅的対応は将来Issueに分離するか、対応する環境変数名の一覧を明記すべき。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "[index] languages セクションの利用箇所が不在", + "description": "現在のインデクサーはファイル拡張子ベースで FileType を判定しており、言語フィルタ設定を受け取る仕組みが存在しない。", + "suggestion": "受け入れ基準に「[index] languages 設定でインデックス対象言語を制限できること」を追加するか、Phase 1 では設定ファイルの読み込みのみとし言語フィルタ適用は別Issueとして分離する。" + }, + { + "id": "S2", + "title": "[search] default_limit / snippet_lines / snippet_chars の反映方法が不明", + "description": "現在 search の limit/snippet_lines/snippet_chars は CLI引数で指定されており、設定ファイルからのデフォルト値上書きが未実装。CLI引数と設定ファイルの優先関係が明記されていない。", + "suggestion": "「CLI引数が明示的に指定された場合はCLI引数を優先、未指定時は設定ファイルの値を使用」という優先順位を明記すべき。" + }, + { + "id": "S3", + "title": "commandindex.toml の配置場所の検出方法が未記載", + "description": "「リポジトリルートに配置」とあるが、検出方法が未記載。既存コードでは --path 引数ベースのディレクトリを使用。", + "suggestion": "--path 引数ベースのディレクトリをリポジトリルートとして扱う方針を明記すべき。" + }, + { + "id": "S4", + "title": "エラー型の設計方針が不足", + "description": "新 config モジュールでは embedding 以外の設定も扱うため、embedding モジュールのエラー型に依存し続けるのは不適切。", + "suggestion": "新 config モジュール用の専用エラー型 ConfigError enum を定義する方針を追加すべき。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "config.local.toml の .gitignore 追加への言及", + "description": ".commandindex/ 自体が gitignore されているなら問題ないが、明示的に言及しておくべき。", + "suggestion": "受け入れ基準に確認項目を追加。" + }, + { + "id": "N2", + "title": "commandindex config show の出力形式", + "description": "出力形式（TOML/JSON/human）が未定義。", + "suggestion": "config show も --format オプションをサポートすることを検討。" + }, + { + "id": "N3", + "title": "config path サブコマンドの詳細仕様", + "description": "何を表示するかが不明確。", + "suggestion": "「読み込まれた設定ファイルのパスを優先順位順に表示する」等の仕様を追加。" + }, + { + "id": "N4", + "title": "マージ用中間構造体の設計", + "description": "フィールドレベルマージには全フィールド Option の中間構造体が必要になる可能性がある。", + "suggestion": "実装方針にマージ戦略の詳細を記載しておくと設計フェーズがスムーズになる。" + } + ], + "summary": "Issue #76 は既存コードベースとの方向性は概ね整合。Must Fix 3件: (1) 既存 embedding::Config との移行計画欠落、(2) マージ粒度未定義、(3) 環境変数スコープ未定義。Should Fix として設定値の消費先との紐付け不足あり。" +} diff --git a/dev-reports/issue/76/issue-review/stage2-apply-result.json b/dev-reports/issue/76/issue-review/stage2-apply-result.json new file mode 100644 index 0000000..b35710f --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage2-apply-result.json @@ -0,0 +1,18 @@ +{ + "stage": 2, + "action": "apply_review", + "applied_items": [ + "M1: 既存 embedding::Config の移行計画を実装方針に追加（対象箇所6箇所を明記）", + "M2: マージ仕様セクションを新設（フィールドレベルマージ、具体例付き）", + "M3: 環境変数オーバーライドのスコープをPhase 1では COMMANDINDEX_OPENAI_API_KEY のみに限定", + "S1: [index] languages はPhase 1では読み込みのみ、インデクサー反映は別Issueと明記", + "S2: CLI引数と設定ファイルの優先関係セクションを追加", + "S3: 設定ファイルの検出方法セクションを追加（--path引数ベース）", + "S4: 専用エラー型 ConfigError を実装方針と受け入れ基準に追加", + "N1: .gitignore確認を受け入れ基準に追加", + "N2: config show の --format オプションを CLI連携に追加", + "N3: config path の仕様を明確化" + ], + "issue_updated": true, + "issue_url": "https://github.com/Kewton/CommandIndex/issues/76" +} diff --git a/dev-reports/issue/76/issue-review/stage3-review-context.json b/dev-reports/issue/76/issue-review/stage3-review-context.json new file mode 100644 index 0000000..e720bc0 --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage3-review-context.json @@ -0,0 +1,75 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "embedding::Config の削除は6箇所の呼び出しサイトに破壊的変更を引き起こす", + "description": "crate::embedding::Config::load() は src/cli/search.rs (4箇所)、src/cli/embed.rs (1箇所)、src/cli/index.rs (1箇所) で直接呼び出されている。一括移行が必要。", + "suggestion": "新 config::AppConfig を作成し全6箇所を1コミットで変更するか、段階的に移行する。" + }, + { + "id": "M2", + "title": "clean.rs での config.toml ハードコード参照の更新が必要", + "description": "src/cli/clean.rs L67, L94 で config.toml をハードコードで参照。新設定ファイル名との追従が必要。", + "suggestion": "clean.rs の保存対象ファイルリストに commandindex.toml と config.local.toml を追加。" + }, + { + "id": "M3", + "title": "EmbeddingConfig と RerankConfig の所有権移動による API 境界変更", + "description": "新 AppConfig がこれらを包含する場合、循環依存に注意が必要。", + "suggestion": "EmbeddingConfig, RerankConfig は現在のモジュールに残し、AppConfig は参照する設計にする。" + }, + { + "id": "M4", + "title": "テストの config.toml 参照が破壊される", + "description": "tests/e2e_embedding.rs と tests/e2e_semantic_hybrid.rs で .commandindex/config.toml をハードコードで作成・検証している。", + "suggestion": "テストを新しい設定ファイルパスに更新し、設定ファイル名を定数化。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "CLI デフォルト値と Config 値の優先順位ロジック", + "description": "clap の default_value_t と config からの値の優先順位が複雑。CLIで明示指定されたかどうかを判別できないと config 値が無視される。", + "suggestion": "clap の default_value_t を使わず Option にして、None の場合に config → ハードコードデフォルトの順でフォールバック。" + }, + { + "id": "S2", + "title": "環境変数の命名規則とプレフィックスの設計", + "description": "既存の COMMANDINDEX_OPENAI_API_KEY との互換性と新しい命名規則の統一が必要。", + "suggestion": "COMMANDINDEX_ プレフィックスで統一し、TOML のキーパスに対応させる。" + }, + { + "id": "S3", + "title": "新エラー型と既存 EmbeddingError::ConfigError の名前衝突", + "description": "新 config::ConfigError と既存 EmbeddingError::ConfigError(String) の関係を整理する必要あり。", + "suggestion": "config::ConfigError を新設し、EmbeddingError::ConfigError は From 変換で生成する形にリファクタリング。" + }, + { + "id": "S4", + "title": "既存 .commandindex/config.toml との関係の明確化", + "description": "commandindex.toml（ルート）と既存の .commandindex/config.toml の関係が曖昧。", + "suggestion": ".commandindex/config.toml は廃止して commandindex.toml に統合するマイグレーションパスを設ける。" + }, + { + "id": "S5", + "title": "config show / config path サブコマンドのテスト追加", + "description": "Commands enum への追加とテストカバレッジが必要。", + "suggestion": "cli_args.rs テストに config show / config path のテストを追加。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "Config::load() の呼び出し回数削減", + "description": "search.rs 内で最大4回呼ばれる可能性がある。config パラメータの引き回しにリファクタリングすべき。", + "suggestion": "AppConfig::load() の結果を1回だけ生成し各関数に引数として渡す。" + }, + { + "id": "N2", + "title": "設定バリデーションの追加", + "description": "AppConfig に validate() メソッドを追加してフィールド値の妥当性をチェック。", + "suggestion": "load 直後にバリデーションを呼び出す設計にする。" + } + ], + "summary": "影響は中規模。最大リスクは embedding::Config の移行（6箇所の呼び出しサイト + clean.rs + 2つの E2E テスト）。CLI デフォルト値との優先順位ロジックは設計上の注意点。新規クレート追加は不要。Rustのコンパイル時チェックにより移行漏れリスクは低いが、E2Eテストと clean.rs のハードコード参照は見落としやすい。" +} diff --git a/dev-reports/issue/76/issue-review/stage4-apply-result.json b/dev-reports/issue/76/issue-review/stage4-apply-result.json new file mode 100644 index 0000000..1362454 --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage4-apply-result.json @@ -0,0 +1,17 @@ +{ + "stage": 4, + "action": "apply_review", + "applied_items": [ + "M1: 6箇所の呼び出しサイト移行は実装方針に既に記載済み（Stage 2で対応済み）", + "M2: clean.rs のハードコード参照更新を実装方針と受け入れ基準に追加", + "M3: EmbeddingConfig/RerankConfig は現在のモジュールに残す設計方針を明記", + "M4: E2Eテストの更新を実装方針と受け入れ基準に追加", + "S1: CLI引数をOptionにする方針を「CLI引数と設定ファイルの優先関係」セクションに追加", + "S3: エラー型の整理方針を実装方針に追加", + "S4: 既存設定ファイルからの移行セクションを新設（.commandindex/config.toml廃止、警告表示）", + "S5: CLIサブコマンドのテスト追加を実装方針に追加", + "N1: Config::load() の呼び出し最適化を実装方針に追加" + ], + "issue_updated": true, + "issue_url": "https://github.com/Kewton/CommandIndex/issues/76" +} diff --git a/dev-reports/issue/76/issue-review/stage5-review-context.json b/dev-reports/issue/76/issue-review/stage5-review-context.json new file mode 100644 index 0000000..63246a9 --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage5-review-context.json @@ -0,0 +1,46 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "config show でシークレットが露出する仕様穴がある", + "description": "commandindex config show で有効設定を表示する際、環境変数由来または設定ファイル由来の api_key を平文表示する危険がある。既存の Debug 実装はマスクしているが、config show の出力仕様にはその配慮が書かれていない。", + "suggestion": "受け入れ基準とCLI仕様に「config show は api_key 等の秘匿値を必ずマスクする」を明記すべき。" + }, + { + "id": "M2", + "title": "旧 .commandindex/config.toml の移行時挙動が未確定", + "description": "新 commandindex.toml が未作成のまま旧ファイルだけ残っているケースで、設定が無視されるのか deprecated fallback として読むのかが未定義。既存ユーザーの動作が突然変わる可能性がある。", + "suggestion": "Phase 1 では旧 .commandindex/config.toml を deprecated fallback として読みつつ警告を出すか、breaking change として明示するかを受け入れ基準に入れるべき。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "設定ファイル検出のベースパス説明が実際のCLI構造と完全には一致していない", + "description": "index/update/embed/clean は --path を持つが、search には --path がなくカレントディレクトリ固定。設定検出のベースパスが曖昧。", + "suggestion": "search 系は Phase 1 では current directory 基準であることを明記するか、別Issueで search に --path を追加すると切り分けるべき。" + }, + { + "id": "S2", + "title": "config path の表示対象に旧設定ファイルを含めるかが未定義", + "description": "旧 .commandindex/config.toml を警告対象にするなら、config path で deprecated path を表示するのか無視するのかが未記載。", + "suggestion": "config path の仕様に旧ファイルの扱いを追記すべき。" + }, + { + "id": "S3", + "title": "受け入れ基準に config show のフォーマット差分が反映されていない", + "description": "CLI仕様では --format toml|json|human まで踏み込んでいるが、受け入れ基準は TOML のみ。", + "suggestion": "Phase 1 のスコープを TOML のみに絞るならCLI仕様から json|human を外すべき。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "設定ファイル名の定数化対象を本体コードにも広げる", + "description": "テスト側だけでなく本体側も clean.rs や新 config モジュール、警告表示で同じファイル名を複数箇所で扱う。", + "suggestion": "TEAM_CONFIG_FILE, LOCAL_CONFIG_FILE, LEGACY_CONFIG_FILE のような定数を定義する方針を追記。" + } + ], + "summary": "1回目レビューの主要指摘は概ね反映済み。2回目の主な懸念は config show のシークレット露出リスクと、旧 .commandindex/config.toml の移行時挙動が未確定な点。設定検出のベースパス説明は search の挙動と完全一致していない。", + "reviewer": "Codex (gpt-5.4 medium)" +} diff --git a/dev-reports/issue/76/issue-review/stage6-apply-result.json b/dev-reports/issue/76/issue-review/stage6-apply-result.json new file mode 100644 index 0000000..791eabe --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage6-apply-result.json @@ -0,0 +1,14 @@ +{ + "stage": 6, + "action": "apply_review", + "applied_items": [ + "M1: config show の秘匿値マスク表示を CLI仕様・実装方針・受け入れ基準に追加", + "M2: 旧 .commandindex/config.toml を deprecated fallback として読み込み維持する方針を明記。優先順位に追加（レベル4）", + "S1: 設定ファイル検出のベースパスをコマンド別に明記（--path持つコマンド vs search/config）", + "S2: config path で旧ファイルを [deprecated] 注記付きで表示する仕様を追加", + "S3: --format オプションは Phase 1 スコープ外と明記、CLI仕様から json/human を除外", + "N1: 設定ファイル名の定数化（TEAM_CONFIG_FILE, LOCAL_CONFIG_FILE, LEGACY_CONFIG_FILE）を実装方針・受け入れ基準に追加" + ], + "issue_updated": true, + "issue_url": "https://github.com/Kewton/CommandIndex/issues/76" +} diff --git a/dev-reports/issue/76/issue-review/stage7-review-context.json b/dev-reports/issue/76/issue-review/stage7-review-context.json new file mode 100644 index 0000000..5c9168c --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage7-review-context.json @@ -0,0 +1,64 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "clean --keep-embeddings の保存対象が新しい設定配置とずれる", + "description": "既存の clean.rs は .commandindex/config.toml を保持対象としてハードコードしている。ルートの commandindex.toml は clean の対象外であり、.commandindex/ 配下で保持すべきなのは config.local.toml と deprecated fallback の config.toml。", + "suggestion": "clean.rs の保持対象を LOCAL_CONFIG_FILE と LEGACY_CONFIG_FILE ベースに更新。ルートの commandindex.toml は clean の対象外であることを明示。回帰テストも追加。" + }, + { + "id": "M2", + "title": "deprecated fallback の警告出力が既存CLI挙動とテストに影響する", + "description": "旧設定ファイルを読む際の警告メッセージの出力先や出力条件が統一されていないと、既存の利用体験やテスト安定性に影響する。", + "suggestion": "警告は stderr に統一し、deprecated fallback を実際に使用した場合だけ1回出す仕様に固定。旧設定のみ存在するケースと新旧両方存在するケースのテストを分けて追加。" + }, + { + "id": "M3", + "title": "config show 実装には Serialize 境界の変更が必要", + "description": "EmbeddingConfig と RerankConfig は Deserialize のみ。config show で TOML 出力するには Serialize が必要。秘匿値マスクには表示用DTOかマスク済み変換が必要。", + "suggestion": "Serialize を追加する対象型と、表示用のマスク済み構造体を分ける方針をIssueに追記。config show 専用の view model を経由させる設計が安全。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "既存機能のベースパス差異に対する回帰確認が必要", + "description": "各コマンドでどのベースパスから設定を探すかのテストが必要。", + "suggestion": "特に search は current_dir を変えたCLIテストで保証する。" + }, + { + "id": "S2", + "title": "CLIデフォルト値を Option に変えることで既存検索系テストの期待値が変わる", + "description": "help表示、引数未指定時の値解決が変わるため、テストは「未指定時はハードコード既定値」「設定あり時は設定値」「CLI明示時はCLI優先」を分けて持つ必要がある。", + "suggestion": "統合テストを追加して優先順位を固定化する。" + }, + { + "id": "S3", + "title": "旧設定を残すことでテスト更新対象は前回想定より広い", + "description": "legacy fallback を維持するため、新パス対応テストに加えて旧ファイルの互換テストも必要。", + "suggestion": "E2Eテストは少なくとも3系統に分ける。commandindex.toml のみ、config.local.toml 上書き、legacy config.toml fallback。" + }, + { + "id": "S4", + "title": "モジュール依存は増える", + "description": "外部クレート追加は不要だが、config モジュール中心の内部依存整理が必要。ConfigError と EmbeddingError::ConfigError の変換設計が曖昧だと責務がねじれやすい。", + "suggestion": "config を唯一の設定読込入口にする原則を明記する。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "設定読込結果の使い回しはパフォーマンス面で妥当", + "description": "AppConfig::load() を1回に寄せる方針でパフォーマンス影響は軽微か改善寄り。", + "suggestion": "パフォーマンス影響の整理を追記。" + }, + { + "id": "N2", + "title": "設定ファイル名の定数化はテスト保守にも効く", + "description": "テストヘルパーでも同じ定数を使う方針にすると将来の移行時にも有効。", + "suggestion": "テストヘルパーでも定数を使う方針を追記。" + } + ], + "summary": "影響範囲は中規模。最大の影響は設定読込入口の集約と clean --keep-embeddings の保持対象変更。テスト影響は前回より広く、legacy fallback と警告出力の互換確認が必要。パフォーマンス影響は小さく改善寄り。外部依存追加は不要だが Serialize 導入と config モジュール中心の内部依存整理が前提。", + "reviewer": "Codex (gpt-5.4 medium)" +} diff --git a/dev-reports/issue/76/issue-review/stage8-apply-result.json b/dev-reports/issue/76/issue-review/stage8-apply-result.json new file mode 100644 index 0000000..900b7aa --- /dev/null +++ b/dev-reports/issue/76/issue-review/stage8-apply-result.json @@ -0,0 +1,15 @@ +{ + "stage": 8, + "action": "apply_review", + "applied_items": [ + "M1: clean --keep-embeddings の保持対象を LOCAL_CONFIG_FILE, LEGACY_CONFIG_FILE に更新。ルートの commandindex.toml は対象外と明記。回帰テストも追加", + "M2: deprecated 警告を stderr に統一、使用した場合のみ1回出力。新旧両方存在時の挙動も明記。受け入れ基準に追加", + "M3: Serialize 対応と config show 用 view model を実装方針に追加。EmbeddingConfig/RerankConfig に Serialize 追加を受け入れ基準に追加", + "S1: 各コマンドのベースパス別設定検出テストを追加", + "S2: CLI引数優先順位テスト3パターンを受け入れ基準に追加", + "S3: E2Eテスト3系統（新設定のみ、ローカル上書き、レガシーfallback）を受け入れ基準に追加", + "S4: config を唯一の設定読込入口にする原則を実装方針に追加" + ], + "issue_updated": true, + "issue_url": "https://github.com/Kewton/CommandIndex/issues/76" +} diff --git a/dev-reports/issue/76/issue-review/summary-report.md b/dev-reports/issue/76/issue-review/summary-report.md new file mode 100644 index 0000000..360937b --- /dev/null +++ b/dev-reports/issue/76/issue-review/summary-report.md @@ -0,0 +1,63 @@ +# マルチステージIssueレビュー サマリーレポート - Issue #76 + +## Issue +- **番号**: #76 +- **タイトル**: [Feature] チーム共有設定ファイル（config.toml） +- **実施日**: 2026-03-22 + +## レビュー概要 + +| Stage | レビュー種別 | 実行エージェント | Must Fix | Should Fix | Nice to Have | +|-------|------------|----------------|----------|------------|--------------| +| 0.5 | 仮説検証 | Claude | - | - | - | +| 1 | 通常レビュー（1回目） | Claude (opus) | 3 | 4 | 4 | +| 2 | 指摘事項反映（1回目） | Claude (sonnet) | 反映完了 | 反映完了 | 反映完了 | +| 3 | 影響範囲レビュー（1回目） | Claude (opus) | 4 | 5 | 2 | +| 4 | 指摘事項反映（1回目） | Claude (sonnet) | 反映完了 | 反映完了 | - | +| 5 | 通常レビュー（2回目） | Codex (gpt-5.4) | 2 | 3 | 1 | +| 6 | 指摘事項反映（2回目） | Claude (sonnet) | 反映完了 | 反映完了 | 反映完了 | +| 7 | 影響範囲レビュー（2回目） | Codex (gpt-5.4) | 3 | 4 | 2 | +| 8 | 指摘事項反映（2回目） | Claude (sonnet) | 反映完了 | 反映完了 | - | + +## 主要な改善点 + +### Stage 1-2 で追加された内容 +1. **マージ仕様の明確化**: フィールドレベルマージの定義と具体例 +2. **環境変数スコープの限定**: Phase 1 は COMMANDINDEX_OPENAI_API_KEY のみ +3. **既存 embedding::Config の移行計画**: 全6箇所の呼び出しサイト特定 + +### Stage 3-4 で追加された内容 +4. **clean.rs のハードコード参照更新**: 新設定ファイル名への追従 +5. **型設計**: EmbeddingConfig/RerankConfig は現モジュールに残す（循環依存回避） +6. **E2Eテスト更新計画**: e2e_embedding.rs, e2e_semantic_hybrid.rs +7. **CLI引数と設定ファイルの優先関係**: Option による実装方針 +8. **Config::load() 呼び出し最適化**: 1回生成・引き回し設計 + +### Stage 5-6 で追加された内容（Codex レビュー） +9. **config show の秘匿値マスク**: api_key 等の平文表示防止 +10. **旧設定ファイルの deprecated fallback**: breaking change 回避 +11. **ベースパス検出のコマンド別明記**: --path 有無による差異 +12. **config path での旧ファイル [deprecated] 注記表示** +13. **--format オプションの Phase 1 スコープ外明記** + +### Stage 7-8 で追加された内容（Codex レビュー） +14. **clean --keep-embeddings の保持対象更新**: LOCAL_CONFIG_FILE, LEGACY_CONFIG_FILE +15. **deprecated 警告の stderr 統一**: 使用時のみ1回出力 +16. **Serialize 対応と view model**: config show 用マスク済み構造体 +17. **E2Eテスト3系統化**: 新設定のみ、ローカル上書き、レガシーfallback +18. **CLI引数優先順位テスト**: 未指定/設定あり/CLI明示の3パターン + +## 仮説検証結果 + +| 仮説 | 判定 | +|------|------| +| config モジュールを新規作成 | Confirmed（未存在） | +| toml crate でパース | Confirmed（既に依存に含まれている） | +| embedding モジュール設定をconfig経由に移行 | Partially Confirmed（既存Config構造体の移行が必要） | + +## 最終 Issue 品質評価 + +- **整合性**: 既存コードベースとの整合性が確保されている +- **網羅性**: 受け入れ基準が20項目に拡充され、テスト要件も明確 +- **実装方針**: 10ステップの具体的な実装手順が定義されている +- **リスク対策**: deprecated fallback、秘匿値マスク、warning 出力統一が対策済み diff --git a/dev-reports/issue/76/multi-stage-design-review/stage1-4-apply-result.json b/dev-reports/issue/76/multi-stage-design-review/stage1-4-apply-result.json new file mode 100644 index 0000000..a92402b --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/stage1-4-apply-result.json @@ -0,0 +1,27 @@ +{ + "stages": "1-4", + "action": "apply_review_to_design_policy", + "must_fix_total": 13, + "applied_items": [ + "Stage1-M1: AppConfig から I/O 分離 → load_config() を公開関数に分離、AppConfig は純粋データ構造に", + "Stage1-M2: RawConfig の DRY 問題 → テストでフィールド同期のラウンドトリップ検証を追加", + "Stage2-M1: RawRerankConfig.provider 削除（既存 RerankConfig に無いフィールド）", + "Stage2-M2: Config::load 呼び出し箇所の正確な行番号を追記（L130, L291, L424, L650）", + "Stage2-M3: clean.rs の保持対象を具体的に列挙（embeddings.db, config.toml, config.local.toml）", + "Stage2-M4: thiserror 不使用に確定、手動 Display + Error 実装に統一", + "Stage3-M1: RawRerankConfig.provider 削除済み（Stage2-M1 と同じ）", + "Stage3-M2: thiserror 不使用確定済み（Stage2-M4 と同じ）", + "Stage3-M3: search.rs の private 関数への &AppConfig 引き回し設計を Section 5.3 に追加", + "Stage3-M4: clean.rs の保持対象ファイルを具体的に列挙済み（Stage2-M3 と同じ）", + "Stage4-M1: RerankConfig に Custom Debug 実装（api_key マスク）を追加", + "Stage4-M2: OpenAiProvider に Custom Debug 実装（api_key マスク）を追加", + "Stage4-M3: チーム設定での api_key 禁止バリデーション validate_no_secrets() を追加", + "追加: AppConfig から Serialize を削除（api_key 露出防止）", + "追加: ConfigSourceKind::Default を削除（YAGNI）", + "追加: CLI help テキストにデフォルト値を明示", + "追加: エラー伝播の From 実装方針を追加", + "追加: ProviderType に Serialize 追加の方針を明記（view model で文字列化に変更）", + "追加: テスト移行計画を追加", + "追加: cli_args.rs の help 出力テスト更新を追記" + ] +} diff --git a/dev-reports/issue/76/multi-stage-design-review/stage1-review-context.json b/dev-reports/issue/76/multi-stage-design-review/stage1-review-context.json new file mode 100644 index 0000000..b052fc2 --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/stage1-review-context.json @@ -0,0 +1,20 @@ +{ + "stage": 1, + "focus": "SOLID/KISS/YAGNI/DRY", + "must_fix": [ + {"id": "M1", "title": "AppConfig has mixed responsibilities: data + I/O + merge + view", "principle": "SRP/OCP", "suggestion": "Extract ConfigLoader for file discovery/reading/merging. AppConfig should be plain data."}, + {"id": "M2", "title": "RawConfig duplicates field definitions from EmbeddingConfig/RerankConfig", "principle": "DRY", "suggestion": "Add compile-time or test-time check that Raw and final structs stay in sync."} + ], + "should_fix": [ + {"id": "S1", "title": "Config::load called 4 times independently in search.rs", "principle": "DRY/KISS", "suggestion": "Load once at top of each CLI handler and pass down."}, + {"id": "S2", "title": "RerankConfig.provider is Option instead of typed enum", "principle": "Type Safety", "suggestion": "Remove provider from RawRerankConfig since only Ollama is supported."}, + {"id": "S3", "title": "No abstraction for config loading makes testing harder", "principle": "DI/Testability", "suggestion": "Accept sources as parameter for testability."}, + {"id": "S4", "title": "AppConfigView duplicates structure for masking", "principle": "DRY", "suggestion": "Use custom Serialize or MaskedField wrapper."} + ], + "nice_to_have": [ + {"id": "N1", "title": "ConfigSourceKind::Default is unused", "principle": "YAGNI"}, + {"id": "N2", "title": "IndexConfig with only languages feels premature", "principle": "YAGNI"}, + {"id": "N3", "title": "SearchConfig defaults lose discoverability from CLI help", "principle": "KISS"}, + {"id": "N4", "title": "EmbeddingError and RerankError have near-identical variants", "principle": "DRY"} + ] +} diff --git a/dev-reports/issue/76/multi-stage-design-review/stage2-review-context.json b/dev-reports/issue/76/multi-stage-design-review/stage2-review-context.json new file mode 100644 index 0000000..aecfbec --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/stage2-review-context.json @@ -0,0 +1,23 @@ +{ + "stage": 2, + "focus": "整合性", + "must_fix": [ + {"id": "M1", "title": "RawRerankConfig has 'provider' field but actual RerankConfig does not", "suggestion": "Remove provider from RawRerankConfig or add to RerankConfig."}, + {"id": "M2", "title": "Config::load call site count needs precise line references", "suggestion": "Clarify: L130, L291, L424, L650 in search.rs."}, + {"id": "M3", "title": "clean.rs preservation list needs config.local.toml added", "suggestion": "Explicitly list: embeddings.db, config.toml (legacy), config.local.toml."}, + {"id": "M4", "title": "ConfigError uses thiserror but it's not a dependency", "suggestion": "Use manual Display+Error implementations to match existing codebase patterns."} + ], + "should_fix": [ + {"id": "S1", "title": "EmbeddingConfig Serialize could expose api_key", "suggestion": "Note that direct serialization must go through masked view model."}, + {"id": "S2", "title": "RerankConfigView missing provider field alignment", "suggestion": "Align after resolving M1."}, + {"id": "S3", "title": "Error propagation from ConfigError to existing error types not addressed", "suggestion": "Add From to IndexError, EmbedError, SearchError."}, + {"id": "S4", "title": "ProviderType needs Serialize derive", "suggestion": "Add Serialize to ProviderType."}, + {"id": "S5", "title": "Import statement changes not listed", "suggestion": "Include import changes in migration plan."} + ], + "nice_to_have": [ + {"id": "N1", "title": "cli/mod.rs module declaration not shown"}, + {"id": "N2", "title": "Default values for SearchConfig not specified"}, + {"id": "N3", "title": "Default values for IndexConfig.languages not specified"}, + {"id": "N4", "title": "Test file paths may be incomplete"} + ] +} diff --git a/dev-reports/issue/76/multi-stage-design-review/stage3-review-context.json b/dev-reports/issue/76/multi-stage-design-review/stage3-review-context.json new file mode 100644 index 0000000..185dd4c --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/stage3-review-context.json @@ -0,0 +1,23 @@ +{ + "stage": 3, + "focus": "影響分析", + "must_fix": [ + {"id": "M1", "title": "RawRerankConfig.provider フィールドが既存と不整合", "suggestion": "RawRerankConfig から provider を削除。"}, + {"id": "M2", "title": "thiserror の依存方針が未確定", "suggestion": "手動実装に確定させる。"}, + {"id": "M3", "title": "search.rs の private 関数への AppConfig 引き回し設計が欠落", "suggestion": "run() で1回ロードし try_hybrid_search, try_rerank に &AppConfig を渡す。"}, + {"id": "M4", "title": "clean.rs の保持対象ファイルの具体的な列挙がない", "suggestion": "embeddings.db, config.toml, config.local.toml を明記。"} + ], + "should_fix": [ + {"id": "S1", "title": "e2e テストの具体的移行方法が未記載", "suggestion": "テスト移行計画を追加。"}, + {"id": "S2", "title": "CLI ヘルプからデフォルト値が消える", "suggestion": "clap help フィールドでデフォルト値を明示。"}, + {"id": "S3", "title": "EmbeddingConfig Serialize 追加による api_key 露出リスク", "suggestion": "AppConfig の Serialize を削除し view model のみに限定。"}, + {"id": "S4", "title": "cli_args.rs テストの更新漏れ", "suggestion": "config サブコマンドのテスト追加を明記。"}, + {"id": "S5", "title": "search コマンドの base_path 決定ロジック未記載", "suggestion": "Path::new(\".\") を明記。"} + ], + "nice_to_have": [ + {"id": "N1", "title": "I/O パフォーマンス改善効果の明記"}, + {"id": "N2", "title": ".gitignore テンプレート提供"}, + {"id": "N3", "title": "IndexConfig.languages の消費者が未定義"}, + {"id": "N4", "title": "deprecated 警告の抑制方法"} + ] +} diff --git a/dev-reports/issue/76/multi-stage-design-review/stage4-review-context.json b/dev-reports/issue/76/multi-stage-design-review/stage4-review-context.json new file mode 100644 index 0000000..5f9dc18 --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/stage4-review-context.json @@ -0,0 +1,21 @@ +{ + "stage": 4, + "focus": "セキュリティ", + "must_fix": [ + {"id": "M1", "title": "RerankConfig derives Debug without masking api_key", "severity": "high", "suggestion": "Custom Debug impl that masks api_key with '***'."}, + {"id": "M2", "title": "OpenAiProvider derives Debug exposing api_key", "severity": "high", "suggestion": "Custom Debug impl for OpenAiProvider."}, + {"id": "M3", "title": "Team config commandindex.toml may contain api_key committed to git", "severity": "high", "suggestion": "Validate that api_key is rejected in commandindex.toml, only allowed in config.local.toml or env vars."} + ], + "should_fix": [ + {"id": "S1", "title": "config.local.toml not explicitly in .gitignore", "severity": "medium", "suggestion": "Add *.local.toml to .gitignore as safety net."}, + {"id": "S2", "title": "No file size limit on config reads", "severity": "low", "suggestion": "Reject files > 1MB."}, + {"id": "S3", "title": "AppConfig Serialize could expose api_key", "severity": "medium", "suggestion": "Remove Serialize from AppConfig, use #[serde(skip)] on api_key."}, + {"id": "S4", "title": "No deny_unknown_fields on RawConfig structs", "severity": "low", "suggestion": "Add deny_unknown_fields or warn on unknown fields."} + ], + "nice_to_have": [ + {"id": "N1", "title": "Secret wrapper type for api_key fields"}, + {"id": "N2", "title": "File permission checks on config.local.toml"}, + {"id": "N3", "title": "TOML parse errors may leak content snippets"}, + {"id": "N4", "title": "No unsafe blocks needed (positive)"} + ] +} diff --git a/dev-reports/issue/76/multi-stage-design-review/summary-report.md b/dev-reports/issue/76/multi-stage-design-review/summary-report.md new file mode 100644 index 0000000..d7355ca --- /dev/null +++ b/dev-reports/issue/76/multi-stage-design-review/summary-report.md @@ -0,0 +1,46 @@ +# マルチステージ設計レビュー サマリーレポート - Issue #76 + +## 概要 +- **Issue**: #76 [Feature] チーム共有設定ファイル（config.toml） +- **実施日**: 2026-03-22 +- **対象**: dev-reports/design/issue-76-team-config-design-policy.md + +## レビュー結果 + +| Stage | レビュー種別 | 実行エージェント | Must Fix | Should Fix | Nice to Have | 状態 | +|-------|------------|----------------|----------|------------|--------------|------| +| 1 | 設計原則（SOLID/KISS/YAGNI/DRY） | Claude (opus) | 2 | 4 | 4 | 完了・反映済 | +| 2 | 整合性 | Claude (opus) | 4 | 5 | 4 | 完了・反映済 | +| 3 | 影響分析 | Claude (opus) | 4 | 5 | 4 | 完了・反映済 | +| 4 | セキュリティ | Claude (opus) | 3 | 4 | 4 | 完了・反映済 | +| 5 | 設計原則（2回目） | Codex | - | - | - | スキップ（セッション問題） | +| 6 | 指摘反映（2回目） | Claude (sonnet) | - | - | - | スキップ | +| 7 | 整合性・影響（2回目） | Codex | - | - | - | スキップ（セッション問題） | +| 8 | 指摘反映（2回目） | Claude (sonnet) | - | - | - | スキップ | + +## 主要な設計改善（Stage 1-4 で反映） + +### SRP / アーキテクチャ +1. `load_config()` を公開関数に分離、`AppConfig` は純粋データ構造に +2. search.rs で `&AppConfig` を引き回す設計を明記 + +### 整合性 +3. `RawRerankConfig` から `provider` フィールドを削除（既存 RerankConfig に無い） +4. `thiserror` 不使用に確定（手動 Display + Error 実装） +5. Config::load 呼び出し箇所の正確な行番号を追記 +6. エラー伝播の `From` 実装方針を追加 + +### セキュリティ +7. `validate_no_secrets()` でチーム設定の api_key を拒否 +8. `RerankConfig` に Custom Debug 実装（api_key マスク） +9. `OpenAiProvider` に Custom Debug 実装（api_key マスク） +10. `AppConfig` から `Serialize` を削除（api_key 露出防止） + +### DRY / YAGNI +11. `ConfigSourceKind::Default` を削除 +12. テストでフィールド同期のラウンドトリップ検証を追加 +13. CLI help テキストにデフォルト値を明示 + +## 最終評価 + +設計方針書は Stage 1-4 の13件の Must Fix 指摘を全て反映済み。主要な設計原則（SRP、DRY、セキュリティ）に準拠した設計となっている。Stage 5-8 は Codex セッションの問題によりスキップしたが、1回目の4段階レビューで十分な品質改善が実現できている。 diff --git a/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-context.json b/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-context.json new file mode 100644 index 0000000..eea142b --- /dev/null +++ b/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-context.json @@ -0,0 +1,19 @@ +{ + "issue_number": 76, + "title": "チーム共有設定ファイル（config.toml）", + "design_policy": "dev-reports/design/issue-76-team-config-design-policy.md", + "work_plan": "dev-reports/issue/76/work-plan.md", + "tasks": [ + "Task 1.1: config モジュールの型定義とエラー型", + "Task 1.2: マージロジックとローダー関数", + "Task 1.3: 既存型への Serialize / Debug 追加", + "Task 2.1: embedding::Config の削除と呼び出し箇所の移行", + "Task 2.2: clean.rs の保持対象更新", + "Task 3.1: config show / config path サブコマンド", + "Task 3.2: 検索引数の Option 化", + "Task 4.1: E2E テスト更新", + "Task 4.2: CLI args テスト更新", + "Task 4.3: config モジュール E2E テスト" + ], + "approach": "TDD - テスト先行で実装" +} diff --git a/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-result.json b/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-result.json new file mode 100644 index 0000000..f7c01a3 --- /dev/null +++ b/dev-reports/issue/76/pm-auto-dev/iteration-1/tdd-result.json @@ -0,0 +1,21 @@ +{ + "status": "success", + "tasks_completed": [ + "Task 1.1: config module types (constants, ConfigError, RawConfig, AppConfig, views)", + "Task 1.2: merge logic and loader (read_toml, validate_no_secrets, merge_raw, resolve_config, load_config)", + "Task 1.3: Serialize/Debug on existing types (ProviderType Serialize, RerankConfig Debug mask, OpenAiProvider Debug mask)", + "Task 2.1: embedding::Config deletion and migration (search.rs, embed.rs, index.rs)", + "Task 2.2: clean.rs preserved files update (LEGACY_CONFIG_FILE constant, config.local.toml added)", + "Task 3.1: config CLI subcommands (config show, config path)", + "Task 3.2: search args Option-ized (--limit, --snippet-lines, --snippet-chars)", + "Task 4.1: e2e test updates (e2e_embedding.rs, e2e_semantic_hybrid.rs)", + "Task 4.2: cli_args.rs tests (config show/path, help, legacy deprecated)", + "Task 4.3: config module unit tests (merge, load, validate, view, roundtrip)" + ], + "tests_added": 30, + "tests_passing": true, + "clippy_clean": true, + "fmt_clean": true, + "total_tests": 433, + "summary": "Implemented hierarchical config resolution (commandindex.toml > config.local.toml > legacy config.toml) with security validation (api_key rejected in team config), masked debug/view output, CLI config show/path subcommands, and Option-ized search args. All 433 tests pass, clippy clean, fmt clean." +} diff --git a/dev-reports/issue/76/work-plan.md b/dev-reports/issue/76/work-plan.md new file mode 100644 index 0000000..b8599a4 --- /dev/null +++ b/dev-reports/issue/76/work-plan.md @@ -0,0 +1,201 @@ +# 作業計画書 - Issue #76: チーム共有設定ファイル（config.toml） + +## Issue概要 + +**Issue番号**: #76 +**タイトル**: [Feature] チーム共有設定ファイル（config.toml） +**サイズ**: L +**優先度**: High +**依存Issue**: なし +**設計方針書**: dev-reports/design/issue-76-team-config-design-policy.md + +--- + +## Phase 1: コアモジュール実装 + +### Task 1.1: config モジュールの型定義とエラー型 + +**成果物**: `src/config/mod.rs`（定数・エラー型・RawConfig・AppConfig・ConfigSource） +**依存**: なし + +- [ ] `src/config/mod.rs` を新規作成 +- [ ] 定数定義: `TEAM_CONFIG_FILE`, `LOCAL_CONFIG_FILE`, `LEGACY_CONFIG_FILE` +- [ ] `ConfigError` enum（手動 Display + Error 実装） + - `ReadError`, `ParseError`, `SerializeError`, `SecretInTeamConfig` +- [ ] `RawConfig` / `RawSearchConfig` / `RawIndexConfig` / `RawEmbeddingConfig` / `RawRerankConfig`（マージ用中間構造体） +- [ ] `AppConfig` / `IndexConfig` / `SearchConfig` / `ConfigSource` / `ConfigSourceKind`（最終設定構造体） +- [ ] `AppConfigView` / `EmbeddingConfigView` / `RerankConfigView`（config show 用 view model） +- [ ] `src/lib.rs` に `pub mod config;` 追加 + +**テスト（TDD: テスト先行）**: +- RawConfig のデフォルト値テスト +- AppConfig の to_masked_view() で api_key がマスクされることを検証 + +### Task 1.2: マージロジックとローダー関数 + +**成果物**: `src/config/mod.rs`（load_config, merge_raw, read_toml, validate_no_secrets, resolve_config） +**依存**: Task 1.1 + +- [ ] `read_toml()`: TOML ファイル読み込み +- [ ] `validate_no_secrets()`: チーム設定の api_key 拒否バリデーション +- [ ] `merge_raw()`: フィールドレベルマージ（higher が優先） +- [ ] `resolve_config()`: RawConfig → AppConfig 変換（デフォルト値適用） +- [ ] `load_config()`: 公開ローダー関数（優先順位に従ったファイル発見・読込・マージ・警告出力） + +**テスト（TDD: テスト先行）**: +- merge_raw: フィールドレベルマージの正確性（base に higher が上書き） +- load_config: ファイルなし → デフォルト値 +- load_config: commandindex.toml のみ → 読み込み成功 +- load_config: config.local.toml の上書き +- load_config: legacy config.toml の deprecated fallback + 警告 +- load_config: 新旧両方存在時の優先順位 +- validate_no_secrets: チーム設定に api_key があればエラー +- RawConfig ↔ AppConfig のフィールド同期ラウンドトリップ検証 + +### Task 1.3: 既存型への Serialize / Debug 追加 + +**成果物**: `src/embedding/mod.rs`, `src/embedding/openai.rs`, `src/rerank/mod.rs` +**依存**: Task 1.1 + +- [ ] `EmbeddingConfig`: 既存の Custom Debug（api_key マスク）を確認 +- [ ] `ProviderType`: `Serialize` derive 追加 +- [ ] `RerankConfig`: Custom Debug 実装（api_key マスク、既存の derive(Debug) を置換） +- [ ] `OpenAiProvider`: Custom Debug 実装（api_key マスク） + +**テスト**: +- RerankConfig の Debug 出力で api_key が "***" になること +- OpenAiProvider の Debug 出力で api_key が "***" になること + +## Phase 2: 既存コード移行 + +### Task 2.1: embedding::Config の削除と呼び出し箇所の移行 + +**成果物**: `src/embedding/mod.rs`, `src/cli/search.rs`, `src/cli/embed.rs`, `src/cli/index.rs` +**依存**: Task 1.2 + +- [ ] `embedding::Config` 構造体と `Config::load()` メソッドを削除 +- [ ] `cli/search.rs`: run() で `load_config()` を1回呼出し、`&AppConfig` を内部関数に引き回し + - L130: rerank_top_resolved → `config.rerank.top_candidates` + - L291: run_semantic_search → `&config` 引数追加 + - L424: try_hybrid_search → `&config` 引数追加 + - L650: try_rerank → `&config` 引数追加 +- [ ] `cli/embed.rs` L110: `Config::load()` → `load_config()` +- [ ] `cli/index.rs` L795: `Config::load()` → `load_config()` +- [ ] 各ファイルの `use crate::embedding::Config` インポートを削除・更新 +- [ ] エラー型に `From` を追加（SearchError, EmbedError, IndexError 等） + +**テスト**: +- 既存テスト（cargo test）が全パスすることを確認 + +### Task 2.2: clean.rs の保持対象更新 + +**成果物**: `src/cli/clean.rs` +**依存**: Task 1.1 + +- [ ] L67, L94 の `config.toml` ハードコード参照を定数（LEGACY_CONFIG_FILE）に更新 +- [ ] 保持対象に `LOCAL_CONFIG_FILE` (`config.local.toml`) を追加 + +**テスト**: +- clean --keep-embeddings で config.toml と config.local.toml が保持されることを確認 + +## Phase 3: CLI サブコマンド追加 + +### Task 3.1: config show / config path サブコマンド + +**成果物**: `src/cli/config.rs`, `src/cli/mod.rs`, `src/main.rs` +**依存**: Task 1.2 + +- [ ] `src/cli/config.rs` を新規作成 + - `run_show()`: load_config → to_masked_view → toml::to_string_pretty → stdout + - `run_path()`: load_config → loaded_sources を優先順位順に表示（legacy は [deprecated] 注記） +- [ ] `src/cli/mod.rs` に `pub mod config;` 追加 +- [ ] `src/main.rs` の Commands enum に `Config { command: ConfigCommands }` 追加 +- [ ] `ConfigCommands` enum: `Show`, `Path` +- [ ] main.rs の match 文に Config ハンドラ追加 + +**テスト（TDD: テスト先行）**: +- config show: TOML 形式で出力、api_key がマスクされていること +- config path: 存在するファイルのパスが表示されること +- config path: legacy ファイルに [deprecated] 注記があること + +### Task 3.2: 検索引数の Option 化 + +**成果物**: `src/main.rs`, `src/cli/search.rs` +**依存**: Task 2.1 + +- [ ] `--limit` を `Option` に変更（help テキストにデフォルト値明示） +- [ ] `--snippet-lines` を `Option` に変更 +- [ ] `--snippet-chars` を `Option` に変更 +- [ ] search.rs の run() で `cli_limit.unwrap_or(config.search.default_limit)` パターンを適用 + +**テスト**: +- CLI引数未指定時: ハードコードデフォルト値（20, 2, 120）が使われること +- 設定ファイルあり時: 設定値が使われること +- CLI引数明示時: CLI値が優先されること + +## Phase 4: テスト更新・追加 + +### Task 4.1: E2E テスト更新 + +**成果物**: `tests/e2e_embedding.rs`, `tests/e2e_semantic_hybrid.rs` +**依存**: Task 2.1 + +- [ ] e2e_embedding.rs: `.commandindex/config.toml` → `commandindex.toml` に変更 +- [ ] e2e_semantic_hybrid.rs: `create_test_config()` を `commandindex.toml` に変更 +- [ ] legacy fallback テスト追加: 旧 `.commandindex/config.toml` のみ存在するケース + +### Task 4.2: CLI args テスト更新 + +**成果物**: `tests/cli_args.rs` +**依存**: Task 3.1 + +- [ ] help 出力に "config" サブコマンドが含まれることを検証 +- [ ] config show / config path の基本動作テスト + +### Task 4.3: config モジュール E2E テスト + +**成果物**: `tests/` 内（新規または既存ファイルに追加） +**依存**: Task 3.1, 3.2 + +- [ ] E2E 3系統: commandindex.toml のみ / config.local.toml 上書き / legacy fallback +- [ ] 各コマンドのベースパス別設定検出テスト（--path 有無） + +## Phase 5: 品質チェックと最終調整 + +### Task 5.1: 品質チェック + +**依存**: 全タスク + +- [ ] `cargo build` エラー0件 +- [ ] `cargo clippy --all-targets -- -D warnings` 警告0件 +- [ ] `cargo test --all` 全テストパス +- [ ] `cargo fmt --all -- --check` 差分なし + +### Task 5.2: deprecated 警告の動作確認 + +- [ ] 旧 config.toml のみ存在: stderr に移行案内が出ること +- [ ] 新旧両方存在: stderr に「旧設定は無視」が出ること +- [ ] 新設定のみ存在: 警告なし + +--- + +## タスク依存関係 + +``` +Task 1.1 ──→ Task 1.2 ──→ Task 2.1 ──→ Task 3.2 ──→ Task 4.3 + │ │ │ + │ │ └──→ Task 4.1 + │ │ + │ └──→ Task 3.1 ──→ Task 4.2 + │ + └──→ Task 1.3 + └──→ Task 2.2 +``` + +## Definition of Done + +- [ ] すべてのタスクが完了 +- [ ] cargo test --all が全パス +- [ ] cargo clippy --all-targets -- -D warnings で警告ゼロ +- [ ] cargo fmt --all -- --check で差分なし +- [ ] 受け入れ基準（Issue #76 の20項目）を全て満たしている diff --git a/src/cli/clean.rs b/src/cli/clean.rs index ca05dcf..9a083dd 100644 --- a/src/cli/clean.rs +++ b/src/cli/clean.rs @@ -1,6 +1,8 @@ use std::fmt; use std::path::Path; +use crate::config::{LEGACY_CONFIG_FILE, LOCAL_CONFIG_FILE}; + #[derive(Debug)] pub enum CleanError { Io(std::io::Error), @@ -64,7 +66,7 @@ pub fn run(path: &Path, options: &CleanOptions) -> Result Result) -> fmt::Result { + match self { + Self::Config(msg) => write!(f, "Config error: {msg}"), + Self::Serialize(msg) => write!(f, "Serialize error: {msg}"), + } + } +} + +impl std::error::Error for ConfigCliError {} + +impl From for ConfigCliError { + fn from(e: ConfigError) -> Self { + Self::Config(e.to_string()) + } +} + +// --------------------------------------------------------------------------- +// config show +// --------------------------------------------------------------------------- + +pub fn run_show() -> Result<(), ConfigCliError> { + let config = load_config(Path::new("."))?; + let view = config.to_masked_view(); + let toml_str = + toml::to_string_pretty(&view).map_err(|e| ConfigCliError::Serialize(e.to_string()))?; + print!("{toml_str}"); + Ok(()) +} + +// --------------------------------------------------------------------------- +// config path +// --------------------------------------------------------------------------- + +pub fn run_path() -> Result<(), ConfigCliError> { + let config = load_config(Path::new("."))?; + + if config.loaded_sources.is_empty() { + println!("No config files loaded (using defaults)."); + } else { + for source in &config.loaded_sources { + let kind_label = match source.kind { + ConfigSourceKind::Team => "[team]", + ConfigSourceKind::Local => "[local]", + ConfigSourceKind::Legacy => "[deprecated]", + }; + println!("{} {}", kind_label, source.path.display()); + } + } + Ok(()) +} diff --git a/src/cli/embed.rs b/src/cli/embed.rs index d82040d..4dd731d 100644 --- a/src/cli/embed.rs +++ b/src/cli/embed.rs @@ -2,8 +2,9 @@ use std::fmt; use std::path::Path; use std::time::{Duration, Instant}; +use crate::config::{ConfigError, load_config}; use crate::embedding::store::{EmbeddingStore, EmbeddingStoreError}; -use crate::embedding::{Config, EmbeddingError, create_provider}; +use crate::embedding::{EmbeddingError, create_provider}; use crate::indexer::manifest::{Manifest, ManifestError}; use crate::indexer::reader::{IndexReaderWrapper, ReaderError}; @@ -19,6 +20,7 @@ pub enum EmbedError { Manifest(ManifestError), Reader(ReaderError), Io(std::io::Error), + Config(String), } impl fmt::Display for EmbedError { @@ -33,6 +35,7 @@ impl fmt::Display for EmbedError { Self::Manifest(e) => write!(f, "Manifest error: {e}"), Self::Reader(e) => write!(f, "Reader error: {e}"), Self::Io(e) => write!(f, "IO error: {e}"), + Self::Config(msg) => write!(f, "Config error: {msg}"), } } } @@ -46,6 +49,7 @@ impl std::error::Error for EmbedError { Self::Manifest(e) => Some(e), Self::Reader(e) => Some(e), Self::Io(e) => Some(e), + Self::Config(_) => None, } } } @@ -80,6 +84,12 @@ impl From for EmbedError { } } +impl From for EmbedError { + fn from(e: ConfigError) -> Self { + Self::Config(e.to_string()) + } +} + // --------------------------------------------------------------------------- // Summary // --------------------------------------------------------------------------- @@ -106,12 +116,11 @@ pub fn run(path: &Path) -> Result { return Err(EmbedError::IndexNotFound); } - // 2. Load config (default if no config.toml) - let config = Config::load(&commandindex_dir)?; - let embedding_config = config.and_then(|c| c.embedding).unwrap_or_default(); + // 2. Load config via new config system + let app_config = load_config(path)?; // 3. Create provider - let provider = create_provider(&embedding_config)?; + let provider = create_provider(&app_config.embedding)?; // 4. Load manifest let manifest = Manifest::load(&commandindex_dir)?; diff --git a/src/cli/index.rs b/src/cli/index.rs index ae741ee..cfd2b39 100644 --- a/src/cli/index.rs +++ b/src/cli/index.rs @@ -4,8 +4,9 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; +use crate::config::{ConfigError, load_config}; use crate::embedding::store::{EmbeddingStore, EmbeddingStoreError}; -use crate::embedding::{Config, EmbeddingError, create_provider}; +use crate::embedding::{EmbeddingError, create_provider}; use crate::indexer::diff::{DiffError, detect_changes, scan_files}; use crate::indexer::manifest::{ self, FileEntry, FileType, Manifest, ManifestError, to_relative_path_string, @@ -34,6 +35,7 @@ pub enum IndexError { IndexNotFound, SchemaVersionMismatch, IndexCorrupted(String), + Config(String), } impl fmt::Display for IndexError { @@ -62,6 +64,7 @@ impl fmt::Display for IndexError { f, "Failed to read index state: {detail}. Run `commandindex clean` then `commandindex index` to rebuild." ), + IndexError::Config(msg) => write!(f, "Config error: {msg}"), } } } @@ -82,7 +85,8 @@ impl std::error::Error for IndexError { IndexError::EmbeddingStore(e) => Some(e), IndexError::IndexNotFound | IndexError::SchemaVersionMismatch - | IndexError::IndexCorrupted(_) => None, + | IndexError::IndexCorrupted(_) + | IndexError::Config(_) => None, } } } @@ -156,6 +160,12 @@ impl From for IndexError { } } +impl From for IndexError { + fn from(e: ConfigError) -> Self { + IndexError::Config(e.to_string()) + } +} + /// インデックスオプション（Default実装で後方互換性を維持） #[derive(Debug, Default)] pub struct IndexOptions { @@ -789,12 +799,11 @@ pub fn run_incremental( /// Embedding生成の共通ロジック（run / run_incremental から呼ばれる） fn generate_embeddings_for_manifest( path: &Path, - commandindex_dir: &Path, + _commandindex_dir: &Path, manifest: &Manifest, ) -> Result<(), IndexError> { - let config = Config::load(commandindex_dir)?; - let embedding_config = config.and_then(|c| c.embedding).unwrap_or_default(); - let provider = create_provider(&embedding_config)?; + let app_config = load_config(path)?; + let provider = create_provider(&app_config.embedding)?; let db_path = crate::indexer::embeddings_db_path(path); let store = EmbeddingStore::open(&db_path)?; diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 2b8cd5c..fd9721e 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,4 +1,5 @@ pub mod clean; +pub mod config; pub mod context; pub mod embed; pub mod index; diff --git a/src/cli/search.rs b/src/cli/search.rs index 2b0a33f..d9b1911 100644 --- a/src/cli/search.rs +++ b/src/cli/search.rs @@ -1,6 +1,7 @@ use std::fmt; use std::path::Path; +use crate::config::{AppConfig, ConfigError, load_config}; use crate::indexer::reader::{IndexReaderWrapper, ReaderError, SearchFilters, SearchOptions}; use crate::indexer::symbol_store::{SymbolInfo, SymbolStore, SymbolStoreError}; use crate::output::{ @@ -19,6 +20,7 @@ pub enum SearchError { RelatedSearch(crate::search::related::RelatedSearchError), Embedding(crate::embedding::EmbeddingError), NoEmbeddings, + Config(String), } impl fmt::Display for SearchError { @@ -54,6 +56,7 @@ impl fmt::Display for SearchError { SearchError::NoEmbeddings => { write!(f, "No embeddings found. Run `commandindex embed` first.") } + SearchError::Config(msg) => write!(f, "Config error: {msg}"), } } } @@ -71,6 +74,7 @@ impl std::error::Error for SearchError { SearchError::RelatedSearch(e) => Some(e), SearchError::Embedding(e) => Some(e), SearchError::NoEmbeddings => None, + SearchError::Config(_) => None, } } } @@ -108,6 +112,12 @@ impl From for SearchError { } } +impl From for SearchError { + fn from(e: ConfigError) -> Self { + SearchError::Config(e.to_string()) + } +} + pub fn run( options: &SearchOptions, filters: &SearchFilters, @@ -122,18 +132,12 @@ pub fn run( } let reader = IndexReaderWrapper::open(&tantivy_dir)?; + // Load config once + let config = load_config(Path::new("."))?; + // rerank有効時、検索前にlimitを拡大して候補を多く取得 let original_limit = options.limit; - let rerank_top_resolved = rerank_top.unwrap_or_else(|| { - // CLI未指定時はconfig.toml → デフォルト20の順で解決 - let commandindex_dir = crate::indexer::commandindex_dir(Path::new(".")); - crate::embedding::Config::load(&commandindex_dir) - .ok() - .flatten() - .and_then(|c| c.rerank) - .map(|r| r.top_candidates) - .unwrap_or(20) - }); + let rerank_top_resolved = rerank_top.unwrap_or(config.rerank.top_candidates); let effective_options = if rerank { let mut opts = options.clone(); opts.limit = std::cmp::max(options.limit, rerank_top_resolved); @@ -149,19 +153,18 @@ pub fn run( let use_hybrid = !effective_options.no_semantic && effective_options.heading.is_none(); let final_results = if use_hybrid { - try_hybrid_search(results, &effective_options, filters)? + try_hybrid_search(results, &effective_options, filters, &config)? } else { results }; // Reranking適用 let final_results = if rerank { - let commandindex_dir = crate::indexer::commandindex_dir(Path::new(".")); let reranked = try_rerank( final_results, &effective_options.query, rerank_top_resolved, - &commandindex_dir, + &config, ); reranked.into_iter().take(original_limit).collect() } else { @@ -286,11 +289,9 @@ pub fn run_semantic_search( return Err(SearchError::SymbolDbNotFound); } - // Load embedding config - let commandindex_dir = crate::indexer::commandindex_dir(Path::new(".")); - let config = crate::embedding::Config::load(&commandindex_dir)?; - let embedding_config = config.and_then(|c| c.embedding).unwrap_or_default(); - let provider = crate::embedding::create_provider(&embedding_config)?; + // Load embedding config via new config system + let config = load_config(Path::new("."))?; + let provider = crate::embedding::create_provider(&config.embedding)?; // Check embeddings exist let store = SymbolStore::open(&db_path)?; @@ -390,6 +391,7 @@ fn try_hybrid_search( bm25_results: Vec, options: &SearchOptions, filters: &SearchFilters, + config: &AppConfig, ) -> Result, SearchError> { use crate::search::hybrid::{HYBRID_OVERSAMPLING_FACTOR, rrf_merge}; @@ -420,16 +422,7 @@ fn try_hybrid_search( } // 3. EmbeddingConfig読み込み → provider生成 - let commandindex_dir = crate::indexer::commandindex_dir(Path::new(".")); - let config = match crate::embedding::Config::load(&commandindex_dir) { - Ok(c) => c, - Err(_) => { - eprintln!("[hybrid] Failed to load embedding config, using BM25 only."); - return Ok(bm25_results); - } - }; - let embedding_config = config.and_then(|c| c.embedding).unwrap_or_default(); - let provider = match crate::embedding::create_provider(&embedding_config) { + let provider = match crate::embedding::create_provider(&config.embedding) { Ok(p) => p, Err(_) => { eprintln!("[hybrid] Failed to create embedding provider, using BM25 only."); @@ -644,20 +637,13 @@ fn try_rerank( results: Vec, query: &str, rerank_top: usize, - commandindex_dir: &Path, + config: &AppConfig, ) -> Vec { - // 1. Config読み込み - let rerank_config = match crate::embedding::Config::load(commandindex_dir) { - Ok(Some(config)) => config.rerank.unwrap_or_default(), - Ok(None) => crate::rerank::RerankConfig::default(), - Err(e) => { - eprintln!("[rerank] Failed to load config: {e}"); - return results; - } - }; + // 1. Use config's rerank settings + let rerank_config = &config.rerank; // 2. Provider生成 - let provider = match crate::rerank::ollama::create_rerank_provider(&rerank_config) { + let provider = match crate::rerank::ollama::create_rerank_provider(rerank_config) { Ok(p) => p, Err(e) => { eprintln!("[rerank] Failed to create provider: {e}"); diff --git a/src/config/mod.rs b/src/config/mod.rs new file mode 100644 index 0000000..6d2a087 --- /dev/null +++ b/src/config/mod.rs @@ -0,0 +1,969 @@ +use std::fmt; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use crate::embedding::{EmbeddingConfig, ProviderType}; +use crate::rerank::RerankConfig; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/// Team-shared config file (repository root) +pub const TEAM_CONFIG_FILE: &str = "commandindex.toml"; +/// Local personal config file (under .commandindex/) +pub const LOCAL_CONFIG_FILE: &str = "config.local.toml"; +/// Legacy config file (deprecated fallback) +pub const LEGACY_CONFIG_FILE: &str = "config.toml"; + +// --------------------------------------------------------------------------- +// ConfigError +// --------------------------------------------------------------------------- + +#[derive(Debug)] +pub enum ConfigError { + ReadError { + path: PathBuf, + source: std::io::Error, + }, + ParseError { + path: PathBuf, + source: toml::de::Error, + }, + SerializeError(toml::ser::Error), + /// Team config contains api_key (security violation) + SecretInTeamConfig { + path: PathBuf, + field: String, + }, +} + +impl fmt::Display for ConfigError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::ReadError { path, source } => { + write!( + f, + "Failed to read config file '{}': {}", + path.display(), + source + ) + } + Self::ParseError { path, source } => { + write!( + f, + "Failed to parse config file '{}': {}", + path.display(), + source + ) + } + Self::SerializeError(e) => write!(f, "Failed to serialize config: {}", e), + Self::SecretInTeamConfig { path, field } => write!( + f, + "Security: '{}' contains '{}'. API keys must be in config.local.toml or environment variables.", + path.display(), + field + ), + } + } +} + +impl std::error::Error for ConfigError {} + +// --------------------------------------------------------------------------- +// RawConfig (intermediate structs for merging, all fields Option) +// --------------------------------------------------------------------------- + +#[derive(Debug, Default, Deserialize)] +pub struct RawConfig { + pub index: Option, + pub search: Option, + pub embedding: Option, + pub rerank: Option, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawSearchConfig { + pub default_limit: Option, + pub snippet_lines: Option, + pub snippet_chars: Option, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawIndexConfig { + pub languages: Option>, +} + +#[derive(Debug, Default, Deserialize)] +pub struct RawEmbeddingConfig { + pub provider: Option, + pub model: Option, + pub endpoint: Option, + pub api_key: Option, +} + +/// RawRerankConfig for merge (no provider field - Ollama is fixed) +#[derive(Debug, Default, Deserialize)] +pub struct RawRerankConfig { + pub model: Option, + pub top_candidates: Option, + pub endpoint: Option, + pub api_key: Option, + pub timeout_secs: Option, +} + +// --------------------------------------------------------------------------- +// AppConfig (final merged config - NO Serialize for security) +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone)] +pub struct AppConfig { + pub index: IndexConfig, + pub search: SearchConfig, + pub embedding: EmbeddingConfig, + pub rerank: RerankConfig, + /// Loaded config file path information + pub loaded_sources: Vec, +} + +#[derive(Debug, Clone)] +pub struct ConfigSource { + pub path: PathBuf, + pub kind: ConfigSourceKind, +} + +#[derive(Debug, Clone)] +pub enum ConfigSourceKind { + Team, // commandindex.toml + Local, // .commandindex/config.local.toml + Legacy, // .commandindex/config.toml (deprecated) +} + +#[derive(Debug, Clone, Serialize)] +pub struct IndexConfig { + pub languages: Vec, +} + +#[derive(Debug, Clone, Serialize)] +pub struct SearchConfig { + pub default_limit: usize, // default: 20 + pub snippet_lines: usize, // default: 2 + pub snippet_chars: usize, // default: 120 +} + +// --------------------------------------------------------------------------- +// View models for config show (Serialize allowed on these only) +// --------------------------------------------------------------------------- + +#[derive(Serialize)] +pub struct AppConfigView { + pub index: IndexConfig, + pub search: SearchConfig, + pub embedding: EmbeddingConfigView, + pub rerank: RerankConfigView, +} + +#[derive(Serialize)] +pub struct EmbeddingConfigView { + pub provider: String, + pub model: String, + pub endpoint: String, + pub api_key: String, +} + +#[derive(Serialize)] +pub struct RerankConfigView { + pub model: String, + pub top_candidates: usize, + pub endpoint: String, + pub api_key: String, + pub timeout_secs: u64, +} + +impl AppConfig { + /// Create a masked view model for config show output + pub fn to_masked_view(&self) -> AppConfigView { + let provider_str = match self.embedding.provider { + ProviderType::Ollama => "ollama", + ProviderType::OpenAi => "openai", + }; + AppConfigView { + index: self.index.clone(), + search: self.search.clone(), + embedding: EmbeddingConfigView { + provider: provider_str.to_string(), + model: self.embedding.model.clone(), + endpoint: self.embedding.endpoint.clone(), + api_key: if self.embedding.api_key.is_some() { + "***".to_string() + } else { + "(not set)".to_string() + }, + }, + rerank: RerankConfigView { + model: self.rerank.model.clone(), + top_candidates: self.rerank.top_candidates, + endpoint: self.rerank.endpoint.clone(), + api_key: if self.rerank.api_key.is_some() { + "***".to_string() + } else { + "(not set)".to_string() + }, + timeout_secs: self.rerank.timeout_secs, + }, + } + } +} + +// --------------------------------------------------------------------------- +// Loader functions +// --------------------------------------------------------------------------- + +/// Load config with priority: env > local > team > legacy > defaults +/// +/// base_path determination: +/// - Commands with --path (index, update, embed, clean): --path value +/// - Commands without --path (search, config): current directory "." +pub fn load_config(base_path: &Path) -> Result { + let mut sources = Vec::new(); + let mut merged = RawConfig::default(); + + let legacy_path = base_path + .join(crate::INDEX_DIR_NAME) + .join(LEGACY_CONFIG_FILE); + let team_path = base_path.join(TEAM_CONFIG_FILE); + let local_path = base_path + .join(crate::INDEX_DIR_NAME) + .join(LOCAL_CONFIG_FILE); + + // 1. Legacy config file (deprecated fallback) + if legacy_path.exists() { + if team_path.exists() { + eprintln!( + "Warning: {} is ignored because {} exists.", + legacy_path.display(), + team_path.display() + ); + } else { + let raw = read_toml(&legacy_path)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { + path: legacy_path, + kind: ConfigSourceKind::Legacy, + }); + eprintln!( + "Warning: {} is deprecated. Please migrate to {}", + sources.last().unwrap().path.display(), + team_path.display() + ); + } + } + + // 2. Team shared config (with api_key validation) + if team_path.exists() { + let raw = read_toml(&team_path)?; + validate_no_secrets(&team_path, &raw)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { + path: team_path, + kind: ConfigSourceKind::Team, + }); + } + + // 3. Local personal config + if local_path.exists() { + let raw = read_toml(&local_path)?; + merged = merge_raw(merged, raw); + sources.push(ConfigSource { + path: local_path, + kind: ConfigSourceKind::Local, + }); + } + + // 4. Convert RawConfig -> AppConfig with defaults + Ok(resolve_config(merged, sources)) +} + +/// Read a TOML file into RawConfig +fn read_toml(path: &Path) -> Result { + let content = std::fs::read_to_string(path).map_err(|e| ConfigError::ReadError { + path: path.to_path_buf(), + source: e, + })?; + toml::from_str(&content).map_err(|e| ConfigError::ParseError { + path: path.to_path_buf(), + source: e, + }) +} + +/// Validate that team config does not contain api_key +fn validate_no_secrets(path: &Path, raw: &RawConfig) -> Result<(), ConfigError> { + if let Some(ref emb) = raw.embedding + && emb.api_key.is_some() + { + return Err(ConfigError::SecretInTeamConfig { + path: path.to_path_buf(), + field: "embedding.api_key".to_string(), + }); + } + if let Some(ref rer) = raw.rerank + && rer.api_key.is_some() + { + return Err(ConfigError::SecretInTeamConfig { + path: path.to_path_buf(), + field: "rerank.api_key".to_string(), + }); + } + Ok(()) +} + +/// Field-level merge: higher priority wins +fn merge_raw(base: RawConfig, higher: RawConfig) -> RawConfig { + RawConfig { + index: merge_index(base.index, higher.index), + search: merge_search(base.search, higher.search), + embedding: merge_embedding(base.embedding, higher.embedding), + rerank: merge_rerank(base.rerank, higher.rerank), + } +} + +fn merge_index( + base: Option, + higher: Option, +) -> Option { + match (base, higher) { + (None, None) => None, + (Some(b), None) => Some(b), + (None, Some(h)) => Some(h), + (Some(b), Some(h)) => Some(RawIndexConfig { + languages: h.languages.or(b.languages), + }), + } +} + +fn merge_search( + base: Option, + higher: Option, +) -> Option { + match (base, higher) { + (None, None) => None, + (Some(b), None) => Some(b), + (None, Some(h)) => Some(h), + (Some(b), Some(h)) => Some(RawSearchConfig { + default_limit: h.default_limit.or(b.default_limit), + snippet_lines: h.snippet_lines.or(b.snippet_lines), + snippet_chars: h.snippet_chars.or(b.snippet_chars), + }), + } +} + +fn merge_embedding( + base: Option, + higher: Option, +) -> Option { + match (base, higher) { + (None, None) => None, + (Some(b), None) => Some(b), + (None, Some(h)) => Some(h), + (Some(b), Some(h)) => Some(RawEmbeddingConfig { + provider: h.provider.or(b.provider), + model: h.model.or(b.model), + endpoint: h.endpoint.or(b.endpoint), + api_key: h.api_key.or(b.api_key), + }), + } +} + +fn merge_rerank( + base: Option, + higher: Option, +) -> Option { + match (base, higher) { + (None, None) => None, + (Some(b), None) => Some(b), + (None, Some(h)) => Some(h), + (Some(b), Some(h)) => Some(RawRerankConfig { + model: h.model.or(b.model), + top_candidates: h.top_candidates.or(b.top_candidates), + endpoint: h.endpoint.or(b.endpoint), + api_key: h.api_key.or(b.api_key), + timeout_secs: h.timeout_secs.or(b.timeout_secs), + }), + } +} + +/// Convert RawConfig to AppConfig with defaults applied +fn resolve_config(raw: RawConfig, sources: Vec) -> AppConfig { + let index = IndexConfig { + languages: raw.index.and_then(|i| i.languages).unwrap_or_default(), + }; + + let search = SearchConfig { + default_limit: raw + .search + .as_ref() + .and_then(|s| s.default_limit) + .unwrap_or(20), + snippet_lines: raw + .search + .as_ref() + .and_then(|s| s.snippet_lines) + .unwrap_or(2), + snippet_chars: raw.search.and_then(|s| s.snippet_chars).unwrap_or(120), + }; + + let embedding = if let Some(emb) = raw.embedding { + EmbeddingConfig { + provider: emb.provider.unwrap_or_default(), + model: emb.model.unwrap_or_else(|| "nomic-embed-text".to_string()), + endpoint: emb + .endpoint + .unwrap_or_else(|| "http://localhost:11434".to_string()), + api_key: emb.api_key, + } + } else { + EmbeddingConfig::default() + }; + + let rerank = if let Some(rer) = raw.rerank { + RerankConfig { + model: rer.model.unwrap_or_else(|| "llama3".to_string()), + top_candidates: rer.top_candidates.unwrap_or(20), + endpoint: rer + .endpoint + .unwrap_or_else(|| "http://localhost:11434".to_string()), + api_key: rer.api_key, + timeout_secs: rer.timeout_secs.unwrap_or(30), + } + } else { + RerankConfig::default() + }; + + AppConfig { + index, + search, + embedding, + rerank, + loaded_sources: sources, + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + // --- RawConfig defaults --- + + #[test] + fn test_raw_config_default_is_all_none() { + let raw = RawConfig::default(); + assert!(raw.index.is_none()); + assert!(raw.search.is_none()); + assert!(raw.embedding.is_none()); + assert!(raw.rerank.is_none()); + } + + // --- ConfigError Display --- + + #[test] + fn test_config_error_display_read() { + let err = ConfigError::ReadError { + path: PathBuf::from("/tmp/config.toml"), + source: std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"), + }; + let msg = format!("{err}"); + assert!(msg.contains("Failed to read")); + assert!(msg.contains("/tmp/config.toml")); + } + + #[test] + fn test_config_error_display_secret() { + let err = ConfigError::SecretInTeamConfig { + path: PathBuf::from("commandindex.toml"), + field: "embedding.api_key".to_string(), + }; + let msg = format!("{err}"); + assert!(msg.contains("Security")); + assert!(msg.contains("embedding.api_key")); + assert!(msg.contains("config.local.toml")); + } + + // --- merge_raw --- + + #[test] + fn test_merge_raw_higher_wins() { + let base = RawConfig { + search: Some(RawSearchConfig { + default_limit: Some(10), + snippet_lines: Some(3), + snippet_chars: None, + }), + embedding: Some(RawEmbeddingConfig { + provider: Some(ProviderType::Ollama), + model: Some("base-model".to_string()), + endpoint: None, + api_key: None, + }), + ..RawConfig::default() + }; + let higher = RawConfig { + search: Some(RawSearchConfig { + default_limit: Some(50), + snippet_lines: None, + snippet_chars: Some(200), + }), + embedding: Some(RawEmbeddingConfig { + provider: None, + model: Some("higher-model".to_string()), + endpoint: Some("http://custom:8080".to_string()), + api_key: Some("sk-key".to_string()), + }), + ..RawConfig::default() + }; + + let merged = merge_raw(base, higher); + let search = merged.search.unwrap(); + assert_eq!(search.default_limit, Some(50)); // higher wins + assert_eq!(search.snippet_lines, Some(3)); // base preserved + assert_eq!(search.snippet_chars, Some(200)); // higher wins + + let emb = merged.embedding.unwrap(); + assert_eq!(emb.provider, Some(ProviderType::Ollama)); // base preserved + assert_eq!(emb.model.as_deref(), Some("higher-model")); // higher wins + assert_eq!(emb.endpoint.as_deref(), Some("http://custom:8080")); // higher wins + assert_eq!(emb.api_key.as_deref(), Some("sk-key")); // higher wins + } + + #[test] + fn test_merge_raw_both_none() { + let base = RawConfig::default(); + let higher = RawConfig::default(); + let merged = merge_raw(base, higher); + assert!(merged.index.is_none()); + assert!(merged.search.is_none()); + assert!(merged.embedding.is_none()); + assert!(merged.rerank.is_none()); + } + + #[test] + fn test_merge_raw_rerank_fields() { + let base = RawConfig { + rerank: Some(RawRerankConfig { + model: Some("base-model".to_string()), + top_candidates: Some(10), + endpoint: None, + api_key: None, + timeout_secs: Some(60), + }), + ..RawConfig::default() + }; + let higher = RawConfig { + rerank: Some(RawRerankConfig { + model: None, + top_candidates: Some(30), + endpoint: Some("http://custom:1234".to_string()), + api_key: Some("key".to_string()), + timeout_secs: None, + }), + ..RawConfig::default() + }; + let merged = merge_raw(base, higher); + let rer = merged.rerank.unwrap(); + assert_eq!(rer.model.as_deref(), Some("base-model")); // base preserved + assert_eq!(rer.top_candidates, Some(30)); // higher wins + assert_eq!(rer.endpoint.as_deref(), Some("http://custom:1234")); + assert_eq!(rer.api_key.as_deref(), Some("key")); + assert_eq!(rer.timeout_secs, Some(60)); // base preserved + } + + // --- validate_no_secrets --- + + #[test] + fn test_validate_no_secrets_clean() { + let raw = RawConfig { + embedding: Some(RawEmbeddingConfig { + provider: Some(ProviderType::Ollama), + model: Some("model".to_string()), + endpoint: None, + api_key: None, + }), + ..RawConfig::default() + }; + let result = validate_no_secrets(Path::new("commandindex.toml"), &raw); + assert!(result.is_ok()); + } + + #[test] + fn test_validate_no_secrets_embedding_api_key_rejected() { + let raw = RawConfig { + embedding: Some(RawEmbeddingConfig { + api_key: Some("sk-secret".to_string()), + ..RawEmbeddingConfig::default() + }), + ..RawConfig::default() + }; + let result = validate_no_secrets(Path::new("commandindex.toml"), &raw); + assert!(result.is_err()); + let err = result.unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("embedding.api_key")); + } + + #[test] + fn test_validate_no_secrets_rerank_api_key_rejected() { + let raw = RawConfig { + rerank: Some(RawRerankConfig { + api_key: Some("sk-secret".to_string()), + ..RawRerankConfig::default() + }), + ..RawConfig::default() + }; + let result = validate_no_secrets(Path::new("commandindex.toml"), &raw); + assert!(result.is_err()); + let err = result.unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("rerank.api_key")); + } + + // --- resolve_config --- + + #[test] + fn test_resolve_config_defaults() { + let raw = RawConfig::default(); + let config = resolve_config(raw, vec![]); + assert_eq!(config.search.default_limit, 20); + assert_eq!(config.search.snippet_lines, 2); + assert_eq!(config.search.snippet_chars, 120); + assert_eq!(config.embedding.provider, ProviderType::Ollama); + assert_eq!(config.embedding.model, "nomic-embed-text"); + assert_eq!(config.embedding.endpoint, "http://localhost:11434"); + assert!(config.embedding.api_key.is_none()); + assert_eq!(config.rerank.model, "llama3"); + assert_eq!(config.rerank.top_candidates, 20); + assert_eq!(config.rerank.timeout_secs, 30); + assert!(config.loaded_sources.is_empty()); + } + + #[test] + fn test_resolve_config_with_values() { + let raw = RawConfig { + search: Some(RawSearchConfig { + default_limit: Some(50), + snippet_lines: None, + snippet_chars: Some(200), + }), + embedding: Some(RawEmbeddingConfig { + provider: Some(ProviderType::OpenAi), + model: Some("text-embedding-3-small".to_string()), + endpoint: Some("https://api.openai.com".to_string()), + api_key: Some("sk-key".to_string()), + }), + rerank: Some(RawRerankConfig { + model: Some("gemma2".to_string()), + top_candidates: Some(30), + endpoint: None, + api_key: None, + timeout_secs: Some(60), + }), + ..RawConfig::default() + }; + let config = resolve_config(raw, vec![]); + assert_eq!(config.search.default_limit, 50); + assert_eq!(config.search.snippet_lines, 2); // default + assert_eq!(config.search.snippet_chars, 200); + assert_eq!(config.embedding.provider, ProviderType::OpenAi); + assert_eq!(config.embedding.model, "text-embedding-3-small"); + assert_eq!(config.embedding.api_key.as_deref(), Some("sk-key")); + assert_eq!(config.rerank.model, "gemma2"); + assert_eq!(config.rerank.top_candidates, 30); + assert_eq!(config.rerank.timeout_secs, 60); + } + + // --- to_masked_view --- + + #[test] + fn test_to_masked_view_masks_api_keys() { + let config = AppConfig { + index: IndexConfig { languages: vec![] }, + search: SearchConfig { + default_limit: 20, + snippet_lines: 2, + snippet_chars: 120, + }, + embedding: EmbeddingConfig { + provider: ProviderType::OpenAi, + model: "test-model".to_string(), + endpoint: "https://api.openai.com".to_string(), + api_key: Some("sk-secret-key".to_string()), + }, + rerank: RerankConfig { + model: "llama3".to_string(), + top_candidates: 20, + endpoint: "http://localhost:11434".to_string(), + api_key: Some("sk-rerank-key".to_string()), + timeout_secs: 30, + }, + loaded_sources: vec![], + }; + + let view = config.to_masked_view(); + assert_eq!(view.embedding.api_key, "***"); + assert_eq!(view.rerank.api_key, "***"); + assert_eq!(view.embedding.provider, "openai"); + } + + #[test] + fn test_to_masked_view_no_api_keys() { + let config = AppConfig { + index: IndexConfig { languages: vec![] }, + search: SearchConfig { + default_limit: 20, + snippet_lines: 2, + snippet_chars: 120, + }, + embedding: EmbeddingConfig::default(), + rerank: RerankConfig::default(), + loaded_sources: vec![], + }; + + let view = config.to_masked_view(); + assert_eq!(view.embedding.api_key, "(not set)"); + assert_eq!(view.rerank.api_key, "(not set)"); + } + + // --- load_config with temp directories --- + + #[test] + fn test_load_config_no_files_returns_defaults() { + let tmp = TempDir::new().unwrap(); + let config = load_config(tmp.path()).unwrap(); + assert_eq!(config.search.default_limit, 20); + assert!(config.loaded_sources.is_empty()); + } + + #[test] + fn test_load_config_team_config_only() { + let tmp = TempDir::new().unwrap(); + std::fs::write( + tmp.path().join("commandindex.toml"), + r#" +[search] +default_limit = 50 + +[embedding] +provider = "ollama" +model = "custom-model" +"#, + ) + .unwrap(); + + let config = load_config(tmp.path()).unwrap(); + assert_eq!(config.search.default_limit, 50); + assert_eq!(config.embedding.model, "custom-model"); + assert_eq!(config.loaded_sources.len(), 1); + assert!(matches!( + config.loaded_sources[0].kind, + ConfigSourceKind::Team + )); + } + + #[test] + fn test_load_config_local_overrides_team() { + let tmp = TempDir::new().unwrap(); + std::fs::write( + tmp.path().join("commandindex.toml"), + r#" +[embedding] +provider = "ollama" +model = "team-model" +"#, + ) + .unwrap(); + + let ci_dir = tmp.path().join(".commandindex"); + std::fs::create_dir_all(&ci_dir).unwrap(); + std::fs::write( + ci_dir.join("config.local.toml"), + r#" +[embedding] +model = "local-model" +api_key = "sk-local-key" +"#, + ) + .unwrap(); + + let config = load_config(tmp.path()).unwrap(); + assert_eq!(config.embedding.model, "local-model"); // local wins + assert_eq!(config.embedding.api_key.as_deref(), Some("sk-local-key")); + assert_eq!(config.loaded_sources.len(), 2); + } + + #[test] + fn test_load_config_legacy_fallback() { + let tmp = TempDir::new().unwrap(); + let ci_dir = tmp.path().join(".commandindex"); + std::fs::create_dir_all(&ci_dir).unwrap(); + std::fs::write( + ci_dir.join("config.toml"), + r#" +[embedding] +provider = "ollama" +model = "legacy-model" +"#, + ) + .unwrap(); + + let config = load_config(tmp.path()).unwrap(); + assert_eq!(config.embedding.model, "legacy-model"); + assert_eq!(config.loaded_sources.len(), 1); + assert!(matches!( + config.loaded_sources[0].kind, + ConfigSourceKind::Legacy + )); + } + + #[test] + fn test_load_config_legacy_ignored_when_team_exists() { + let tmp = TempDir::new().unwrap(); + + // Team config + std::fs::write( + tmp.path().join("commandindex.toml"), + r#" +[embedding] +provider = "ollama" +model = "team-model" +"#, + ) + .unwrap(); + + // Legacy config (should be ignored) + let ci_dir = tmp.path().join(".commandindex"); + std::fs::create_dir_all(&ci_dir).unwrap(); + std::fs::write( + ci_dir.join("config.toml"), + r#" +[embedding] +provider = "ollama" +model = "legacy-model" +"#, + ) + .unwrap(); + + let config = load_config(tmp.path()).unwrap(); + assert_eq!(config.embedding.model, "team-model"); // team wins, legacy ignored + assert_eq!(config.loaded_sources.len(), 1); + assert!(matches!( + config.loaded_sources[0].kind, + ConfigSourceKind::Team + )); + } + + #[test] + fn test_load_config_team_with_api_key_rejected() { + let tmp = TempDir::new().unwrap(); + std::fs::write( + tmp.path().join("commandindex.toml"), + r#" +[embedding] +provider = "openai" +api_key = "sk-should-not-be-here" +"#, + ) + .unwrap(); + + let result = load_config(tmp.path()); + assert!(result.is_err()); + let msg = format!("{}", result.unwrap_err()); + assert!(msg.contains("embedding.api_key")); + } + + #[test] + fn test_load_config_invalid_toml() { + let tmp = TempDir::new().unwrap(); + std::fs::write(tmp.path().join("commandindex.toml"), "invalid toml {{{{").unwrap(); + + let result = load_config(tmp.path()); + assert!(result.is_err()); + let msg = format!("{}", result.unwrap_err()); + assert!(msg.contains("Failed to parse")); + } + + // --- TOML roundtrip / field sync test --- + + #[test] + fn test_toml_roundtrip_all_fields() { + let toml_str = r#" +[index] +languages = ["typescript", "python"] + +[search] +default_limit = 50 +snippet_lines = 5 +snippet_chars = 200 + +[embedding] +provider = "openai" +model = "text-embedding-3-small" +endpoint = "https://api.openai.com" +api_key = "sk-test" + +[rerank] +model = "gemma2" +top_candidates = 30 +endpoint = "http://localhost:11434" +api_key = "sk-rerank" +timeout_secs = 60 +"#; + let raw: RawConfig = toml::from_str(toml_str).unwrap(); + let config = resolve_config(raw, vec![]); + + assert_eq!(config.index.languages, vec!["typescript", "python"]); + assert_eq!(config.search.default_limit, 50); + assert_eq!(config.search.snippet_lines, 5); + assert_eq!(config.search.snippet_chars, 200); + assert_eq!(config.embedding.provider, ProviderType::OpenAi); + assert_eq!(config.embedding.model, "text-embedding-3-small"); + assert_eq!(config.embedding.endpoint, "https://api.openai.com"); + assert_eq!(config.embedding.api_key.as_deref(), Some("sk-test")); + assert_eq!(config.rerank.model, "gemma2"); + assert_eq!(config.rerank.top_candidates, 30); + assert_eq!(config.rerank.endpoint, "http://localhost:11434"); + assert_eq!(config.rerank.api_key.as_deref(), Some("sk-rerank")); + assert_eq!(config.rerank.timeout_secs, 60); + } + + // --- view model TOML serialization --- + + #[test] + fn test_view_model_serializes_to_toml() { + let config = AppConfig { + index: IndexConfig { + languages: vec!["rust".to_string()], + }, + search: SearchConfig { + default_limit: 20, + snippet_lines: 2, + snippet_chars: 120, + }, + embedding: EmbeddingConfig { + provider: ProviderType::Ollama, + model: "nomic-embed-text".to_string(), + endpoint: "http://localhost:11434".to_string(), + api_key: Some("secret".to_string()), + }, + rerank: RerankConfig::default(), + loaded_sources: vec![], + }; + let view = config.to_masked_view(); + let toml_str = toml::to_string_pretty(&view).unwrap(); + assert!(toml_str.contains("api_key = \"***\"")); + assert!(!toml_str.contains("secret")); + } +} diff --git a/src/embedding/mod.rs b/src/embedding/mod.rs index c58317a..1a1c309 100644 --- a/src/embedding/mod.rs +++ b/src/embedding/mod.rs @@ -3,10 +3,8 @@ pub mod openai; pub mod store; use std::fmt; -use std::fs; -use std::path::Path; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; // --------------------------------------------------------------------------- // EmbeddingProvider trait @@ -68,7 +66,7 @@ impl std::error::Error for EmbeddingError {} // --------------------------------------------------------------------------- /// プロバイダー種別 -#[derive(Debug, Clone, Default, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "lowercase")] pub enum ProviderType { #[default] @@ -131,32 +129,6 @@ impl EmbeddingConfig { } } -// --------------------------------------------------------------------------- -// Config (top-level config.toml) -// --------------------------------------------------------------------------- - -#[derive(Debug, Clone, Deserialize)] -pub struct Config { - pub embedding: Option, - pub rerank: Option, -} - -impl Config { - /// `.commandindex/config.toml` を読み込む。存在しない場合はNone。 - pub fn load(commandindex_dir: &Path) -> Result, EmbeddingError> { - let config_path = commandindex_dir.join("config.toml"); - if !config_path.exists() { - return Ok(None); - } - let content = fs::read_to_string(&config_path) - .map_err(|e| EmbeddingError::ConfigError(format!("Failed to read config.toml: {e}")))?; - let config: Config = toml::from_str(&content).map_err(|e| { - EmbeddingError::ConfigError(format!("Failed to parse config.toml: {e}")) - })?; - Ok(Some(config)) - } -} - // --------------------------------------------------------------------------- // Shared utilities for providers // --------------------------------------------------------------------------- @@ -262,7 +234,6 @@ impl EmbeddingProvider for MockProvider { #[cfg(test)] mod tests { use super::*; - use tempfile::TempDir; // --- ProviderType / EmbeddingConfig defaults --- @@ -300,19 +271,17 @@ mod tests { assert!(debug_str.contains("None")); } - // --- TOML parsing --- + // --- TOML parsing (EmbeddingConfig directly) --- #[test] - fn test_config_parse_full_toml() { + fn test_embedding_config_toml_parse() { let toml_str = r#" -[embedding] provider = "openai" model = "text-embedding-3-small" endpoint = "https://api.openai.com" api_key = "sk-test" "#; - let config: Config = toml::from_str(toml_str).unwrap(); - let emb = config.embedding.unwrap(); + let emb: EmbeddingConfig = toml::from_str(toml_str).unwrap(); assert_eq!(emb.provider, ProviderType::OpenAi); assert_eq!(emb.model, "text-embedding-3-small"); assert_eq!(emb.endpoint, "https://api.openai.com"); @@ -320,97 +289,17 @@ api_key = "sk-test" } #[test] - fn test_config_parse_minimal_toml() { + fn test_embedding_config_toml_minimal() { let toml_str = r#" -[embedding] provider = "ollama" "#; - let config: Config = toml::from_str(toml_str).unwrap(); - let emb = config.embedding.unwrap(); + let emb: EmbeddingConfig = toml::from_str(toml_str).unwrap(); assert_eq!(emb.provider, ProviderType::Ollama); assert_eq!(emb.model, "nomic-embed-text"); assert_eq!(emb.endpoint, "http://localhost:11434"); assert!(emb.api_key.is_none()); } - #[test] - fn test_config_parse_no_embedding_section() { - let toml_str = ""; - let config: Config = toml::from_str(toml_str).unwrap(); - assert!(config.embedding.is_none()); - assert!(config.rerank.is_none()); - } - - #[test] - fn test_config_parse_with_rerank_section() { - let toml_str = r#" -[embedding] -provider = "ollama" - -[rerank] -model = "gemma2" -top_candidates = 30 -endpoint = "http://localhost:11434" -timeout_secs = 60 -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let rerank = config.rerank.unwrap(); - assert_eq!(rerank.model, "gemma2"); - assert_eq!(rerank.top_candidates, 30); - assert_eq!(rerank.endpoint, "http://localhost:11434"); - assert_eq!(rerank.timeout_secs, 60); - assert!(rerank.api_key.is_none()); - } - - #[test] - fn test_config_parse_rerank_defaults() { - let toml_str = r#" -[rerank] -"#; - let config: Config = toml::from_str(toml_str).unwrap(); - let rerank = config.rerank.unwrap(); - assert_eq!(rerank.model, "llama3"); - assert_eq!(rerank.top_candidates, 20); - assert_eq!(rerank.endpoint, "http://localhost:11434"); - assert_eq!(rerank.timeout_secs, 30); - } - - // --- Config::load --- - - #[test] - fn test_config_load_nonexistent_returns_none() { - let tmp = TempDir::new().unwrap(); - let result = Config::load(tmp.path()).unwrap(); - assert!(result.is_none()); - } - - #[test] - fn test_config_load_valid_file() { - let tmp = TempDir::new().unwrap(); - let config_path = tmp.path().join("config.toml"); - fs::write( - &config_path, - r#" -[embedding] -provider = "ollama" -model = "nomic-embed-text" -"#, - ) - .unwrap(); - let config = Config::load(tmp.path()).unwrap().unwrap(); - let emb = config.embedding.unwrap(); - assert_eq!(emb.provider, ProviderType::Ollama); - } - - #[test] - fn test_config_load_invalid_toml() { - let tmp = TempDir::new().unwrap(); - let config_path = tmp.path().join("config.toml"); - fs::write(&config_path, "not valid toml {{{{").unwrap(); - let result = Config::load(tmp.path()); - assert!(result.is_err()); - } - // --- resolve_api_key --- #[test] diff --git a/src/embedding/openai.rs b/src/embedding/openai.rs index b329f89..16d1f2e 100644 --- a/src/embedding/openai.rs +++ b/src/embedding/openai.rs @@ -42,7 +42,6 @@ struct OpenAiEmbeddingData { // --------------------------------------------------------------------------- /// Embedding provider for OpenAI API (and compatible endpoints like Azure OpenAI). -#[derive(Debug)] pub struct OpenAiProvider { api_key: String, model: String, @@ -51,6 +50,16 @@ pub struct OpenAiProvider { cached_dimension: OnceLock, } +impl std::fmt::Debug for OpenAiProvider { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("OpenAiProvider") + .field("api_key", &"***") + .field("model", &self.model) + .field("endpoint", &self.endpoint) + .finish() + } +} + impl OpenAiProvider { /// Create a new OpenAiProvider. pub fn new(api_key: &str, model: &str, endpoint: &str) -> Self { @@ -161,6 +170,19 @@ mod tests { use super::*; use crate::embedding::{EmbeddingConfig, ProviderType}; + #[test] + fn test_openai_provider_debug_masks_api_key() { + let provider = OpenAiProvider::new( + "sk-super-secret-key-12345", + "text-embedding-3-small", + "https://api.openai.com", + ); + let debug_str = format!("{provider:?}"); + assert!(!debug_str.contains("sk-super-secret-key-12345")); + assert!(debug_str.contains("***")); + assert!(debug_str.contains("text-embedding-3-small")); + } + #[test] fn test_from_config_no_api_key_fails() { // SAFETY: test-only, single-threaded test execution via cargo test diff --git a/src/lib.rs b/src/lib.rs index 1448c13..315f989 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ // CommandIndex library root pub mod cli; +pub mod config; pub mod embedding; pub mod indexer; pub mod output; diff --git a/src/main.rs b/src/main.rs index 4c08d61..276be04 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,6 +13,7 @@ struct Cli { } #[derive(Subcommand)] +#[allow(clippy::large_enum_variant)] enum Commands { /// Build search index from repository Index { @@ -59,15 +60,15 @@ enum Commands { /// Filter by heading #[arg(long)] heading: Option, - /// Maximum number of results (1-1000) - #[arg(long, default_value_t = 20)] - limit: usize, - /// Number of snippet lines (0 = unlimited) - #[arg(long, default_value_t = 2)] - snippet_lines: usize, - /// Number of snippet characters for single-line body (0 = unlimited) - #[arg(long, default_value_t = 120)] - snippet_chars: usize, + /// Maximum number of results (default: from config or 20) + #[arg(long)] + limit: Option, + /// Number of snippet lines (default: from config or 2) + #[arg(long)] + snippet_lines: Option, + /// Number of snippet characters for single-line body (default: from config or 120) + #[arg(long)] + snippet_chars: Option, /// Enable LLM-based reranking of search results #[arg(long, conflicts_with_all = ["symbol", "related", "semantic"])] rerank: bool, @@ -122,6 +123,19 @@ enum Commands { #[arg(long, default_value = ".")] path: PathBuf, }, + /// Show or manage configuration + Config { + #[command(subcommand)] + command: ConfigCommands, + }, +} + +#[derive(Subcommand)] +enum ConfigCommands { + /// Show current effective config (secrets masked) + Show, + /// Show loaded config file paths + Path, } fn main() { @@ -170,9 +184,24 @@ fn main() { rerank, rerank_top, } => { + // Resolve optional args with config defaults + let config = commandindex::config::load_config(std::path::Path::new(".")); + let (effective_limit, effective_snippet_lines, effective_snippet_chars) = match &config + { + Ok(cfg) => ( + limit.unwrap_or(cfg.search.default_limit).min(1000), + snippet_lines.unwrap_or(cfg.search.snippet_lines), + snippet_chars.unwrap_or(cfg.search.snippet_chars), + ), + Err(_) => ( + limit.unwrap_or(20).min(1000), + snippet_lines.unwrap_or(2), + snippet_chars.unwrap_or(120), + ), + }; let snippet_config = commandindex::output::SnippetConfig { - lines: snippet_lines, - chars: snippet_chars, + lines: effective_snippet_lines, + chars: effective_snippet_chars, }; let result = match (query, symbol, related, semantic) { (Some(q), None, None, None) => { @@ -180,7 +209,7 @@ fn main() { query: q, tag, heading, - limit: limit.min(1000), + limit: effective_limit, no_semantic, }; let filters = commandindex::indexer::reader::SearchFilters { @@ -190,10 +219,10 @@ fn main() { commandindex::cli::search::run(&options, &filters, format, snippet_config, rerank, rerank_top) } (None, Some(s), None, None) => { - commandindex::cli::search::run_symbol_search(&s, limit.min(1000), format) + commandindex::cli::search::run_symbol_search(&s, effective_limit, format) } (None, None, Some(f), None) => { - commandindex::cli::search::run_related_search(&f, limit.min(1000), format) + commandindex::cli::search::run_related_search(&f, effective_limit, format) } (None, None, None, Some(q)) => { let filters = commandindex::indexer::reader::SearchFilters { @@ -202,7 +231,7 @@ fn main() { }; commandindex::cli::search::run_semantic_search( &q, - limit.min(1000), + effective_limit, format, tag.as_deref(), &filters, @@ -312,6 +341,22 @@ fn main() { 1 } }, + Commands::Config { command } => match command { + ConfigCommands::Show => match commandindex::cli::config::run_show() { + Ok(()) => 0, + Err(e) => { + eprintln!("Error: {e}"); + 1 + } + }, + ConfigCommands::Path => match commandindex::cli::config::run_path() { + Ok(()) => 0, + Err(e) => { + eprintln!("Error: {e}"); + 1 + } + }, + }, }; process::exit(exit_code); diff --git a/src/rerank/mod.rs b/src/rerank/mod.rs index 3538f3a..95c197d 100644 --- a/src/rerank/mod.rs +++ b/src/rerank/mod.rs @@ -24,7 +24,7 @@ fn default_timeout_secs() -> u64 { 30 } -#[derive(Debug, Clone, Deserialize)] +#[derive(Clone, Deserialize)] pub struct RerankConfig { #[serde(default = "default_rerank_model")] pub model: String, @@ -38,6 +38,18 @@ pub struct RerankConfig { pub timeout_secs: u64, } +impl fmt::Debug for RerankConfig { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("RerankConfig") + .field("model", &self.model) + .field("top_candidates", &self.top_candidates) + .field("endpoint", &self.endpoint) + .field("api_key", &self.api_key.as_ref().map(|_| "***")) + .field("timeout_secs", &self.timeout_secs) + .finish() + } +} + impl Default for RerankConfig { fn default() -> Self { Self { @@ -185,6 +197,24 @@ mod tests { assert_eq!(wrapper.rerank.top_candidates, 20); } + #[test] + fn test_rerank_config_debug_masks_api_key() { + let config = RerankConfig { + api_key: Some("sk-secret-rerank-key".to_string()), + ..RerankConfig::default() + }; + let debug_str = format!("{config:?}"); + assert!(!debug_str.contains("sk-secret-rerank-key")); + assert!(debug_str.contains("***")); + } + + #[test] + fn test_rerank_config_debug_no_api_key() { + let config = RerankConfig::default(); + let debug_str = format!("{config:?}"); + assert!(debug_str.contains("None")); + } + #[test] fn test_rerank_error_display() { let err = RerankError::NetworkError("conn refused".to_string()); diff --git a/tests/cli_args.rs b/tests/cli_args.rs index be65980..8bc7d73 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -14,7 +14,8 @@ fn help_flag_shows_usage() { .stdout(predicate::str::contains("update")) .stdout(predicate::str::contains("status")) .stdout(predicate::str::contains("clean")) - .stdout(predicate::str::contains("context")); + .stdout(predicate::str::contains("context")) + .stdout(predicate::str::contains("config")); } #[test] @@ -299,3 +300,109 @@ fn search_rerank_top_requires_rerank() { predicate::str::contains("required").or(predicate::str::contains("can only be used")), ); } + +#[test] +fn config_show_runs_successfully() { + let tmp = tempfile::tempdir().expect("create temp dir"); + common::cmd() + .args(["config", "show"]) + .current_dir(tmp.path()) + .assert() + .success(); +} + +#[test] +fn config_path_runs_successfully() { + let tmp = tempfile::tempdir().expect("create temp dir"); + common::cmd() + .args(["config", "path"]) + .current_dir(tmp.path()) + .assert() + .success() + .stdout(predicate::str::contains("No config files loaded")); +} + +#[test] +fn config_show_with_team_config() { + let tmp = tempfile::tempdir().expect("create temp dir"); + std::fs::write( + tmp.path().join("commandindex.toml"), + "[embedding]\nprovider = \"ollama\"\nmodel = \"custom-model\"\n", + ) + .unwrap(); + common::cmd() + .args(["config", "show"]) + .current_dir(tmp.path()) + .assert() + .success() + .stdout(predicate::str::contains("custom-model")) + .stdout(predicate::str::contains("api_key")); +} + +#[test] +fn config_path_with_team_config() { + let tmp = tempfile::tempdir().expect("create temp dir"); + std::fs::write( + tmp.path().join("commandindex.toml"), + "[embedding]\nprovider = \"ollama\"\n", + ) + .unwrap(); + common::cmd() + .args(["config", "path"]) + .current_dir(tmp.path()) + .assert() + .success() + .stdout(predicate::str::contains("[team]")) + .stdout(predicate::str::contains("commandindex.toml")); +} + +#[test] +fn config_path_legacy_shows_deprecated() { + let tmp = tempfile::tempdir().expect("create temp dir"); + let ci_dir = tmp.path().join(".commandindex"); + std::fs::create_dir_all(&ci_dir).unwrap(); + std::fs::write( + ci_dir.join("config.toml"), + "[embedding]\nprovider = \"ollama\"\n", + ) + .unwrap(); + common::cmd() + .args(["config", "path"]) + .current_dir(tmp.path()) + .assert() + .success() + .stdout(predicate::str::contains("[deprecated]")); +} + +#[test] +fn config_help_shows_subcommands() { + common::cmd() + .args(["config", "--help"]) + .assert() + .success() + .stdout(predicate::str::contains("show")) + .stdout(predicate::str::contains("path")); +} + +#[test] +fn search_limit_is_optional() { + // Verify that search works without explicit --limit (it's now Option) + let tmp = tempfile::tempdir().expect("create temp dir"); + common::cmd() + .current_dir(tmp.path()) + .args(["search", "test query"]) + .assert() + .failure() + .stderr(predicate::str::contains("Index not found")); +} + +#[test] +fn search_with_explicit_limit_accepted() { + let tmp = tempfile::tempdir().expect("create temp dir"); + common::cmd() + .current_dir(tmp.path()) + .args(["search", "test query", "--limit", "10"]) + .assert() + .failure() + .stderr(predicate::str::contains("Index not found")); +} diff --git a/tests/e2e_embedding.rs b/tests/e2e_embedding.rs index 71cedb4..be2c180 100644 --- a/tests/e2e_embedding.rs +++ b/tests/e2e_embedding.rs @@ -32,7 +32,7 @@ fn clean_keep_embeddings_preserves_embeddings_db() { .assert() .success(); - // Create a dummy embeddings.db and config.toml in .commandindex/ + // Create a dummy embeddings.db, config.toml, and config.local.toml in .commandindex/ let commandindex_dir = dir.path().join(".commandindex"); std::fs::write(commandindex_dir.join("embeddings.db"), "dummy").unwrap(); std::fs::write( @@ -40,6 +40,11 @@ fn clean_keep_embeddings_preserves_embeddings_db() { "[embedding]\nprovider = \"ollama\"\n", ) .unwrap(); + std::fs::write( + commandindex_dir.join("config.local.toml"), + "[embedding]\napi_key = \"sk-test\"\n", + ) + .unwrap(); // Clean with --keep-embeddings common::cmd() @@ -53,9 +58,10 @@ fn clean_keep_embeddings_preserves_embeddings_db() { .success() .stdout(predicate::str::contains("embeddings preserved")); - // Verify embeddings.db and config.toml are preserved + // Verify embeddings.db, config.toml, and config.local.toml are preserved assert!(commandindex_dir.join("embeddings.db").exists()); assert!(commandindex_dir.join("config.toml").exists()); + assert!(commandindex_dir.join("config.local.toml").exists()); // Verify tantivy, manifest.json, state.json are removed assert!(!commandindex_dir.join("tantivy").exists()); diff --git a/tests/e2e_semantic_hybrid.rs b/tests/e2e_semantic_hybrid.rs index 5a4bc1d..aa07bfe 100644 --- a/tests/e2e_semantic_hybrid.rs +++ b/tests/e2e_semantic_hybrid.rs @@ -76,7 +76,8 @@ fn insert_test_embeddings( Ok(()) } -/// Create a minimal config.toml for embedding configuration. +/// Create a minimal commandindex.toml for embedding configuration. +/// Placed at the repo root (parent of .commandindex/). fn create_test_config(commandindex_dir: &std::path::Path) -> Result<(), Box> { let config_content = "\ [embedding] @@ -85,7 +86,11 @@ model = \"nomic-embed-text\" endpoint = \"http://localhost:11434\" dimension = 4 "; - fs::write(commandindex_dir.join("config.toml"), config_content)?; + // commandindex_dir is .commandindex/, so parent is the repo root + let base_path = commandindex_dir + .parent() + .expect("commandindex_dir should have parent"); + fs::write(base_path.join("commandindex.toml"), config_content)?; Ok(()) } From 669e7b77e354fc60d09c575a174a996316bb728e Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 22 Mar 2026 19:59:12 +0900 Subject: [PATCH 2/2] =?UTF-8?q?feat(cli):=20=E3=82=A4=E3=83=B3=E3=83=87?= =?UTF-8?q?=E3=83=83=E3=82=AF=E3=82=B9=E5=85=B1=E6=9C=89=E3=83=A2=E3=83=BC?= =?UTF-8?q?=E3=83=89=E5=AE=9F=E8=A3=85=20(#77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit export/import サブコマンドと status --verify オプションを追加。 CI/CDパイプラインでインデックスを事前生成し、チーム共有可能に。 - commandindex export: tar.gz形式でインデックスをエクスポート - commandindex import: tar.gzからインデックスをインポート（--force対応） - commandindex status --verify: インデックス整合性チェック - セキュリティ: パストラバーサル防止、シンボリックリンク拒否、圧縮爆弾対策 - 一時ディレクトリへの展開後にアトミックスワップ（データ保護） Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 2 + Cargo.toml | 2 + dev-reports/design/issue-77-design-policy.md | 452 ++++++++++++++++++ .../issue-review/hypothesis-verification.md | 6 + .../issue/77/issue-review/original-issue.json | 1 + .../issue-review/stage1-review-context.json | 81 ++++ .../77/issue-review/stage2-apply-result.json | 20 + .../issue-review/stage3-review-context.json | 81 ++++ .../77/issue-review/stage4-apply-result.json | 20 + .../issue-review/stage5-review-context.json | 9 + .../77/issue-review/stage6-apply-result.json | 5 + .../issue-review/stage7-review-context.json | 9 + .../77/issue-review/stage8-apply-result.json | 5 + .../issue/77/issue-review/summary-report.md | 52 ++ .../stage1-apply-result.json | 1 + .../stage1-review-context.json | 21 + .../stage2-apply-result.json | 1 + .../stage2-review-context.json | 20 + .../stage3-apply-result.json | 1 + .../stage3-review-context.json | 24 + .../stage4-apply-result.json | 1 + .../stage4-review-context.json | 23 + .../stage5-review-context.json | 1 + .../stage6-apply-result.json | 1 + .../stage7-review-context.json | 1 + .../stage8-apply-result.json | 1 + .../summary-report.md | 43 ++ .../iteration-1/acceptance-result.json | 7 + .../iteration-1/codex-review-result.json | 1 + .../iteration-1/refactor-result.json | 7 + .../pm-auto-dev/iteration-1/tdd-context.json | 18 + .../pm-auto-dev/iteration-1/tdd-result.json | 28 ++ dev-reports/issue/77/work-plan.md | 207 ++++++++ src/cli/export.rs | 196 ++++++++ src/cli/import_index.rs | 375 +++++++++++++++ src/cli/mod.rs | 2 + src/cli/status.rs | 183 ++++++- src/indexer/mod.rs | 1 + src/indexer/snapshot.rs | 120 +++++ src/main.rs | 84 +++- tests/cli_args.rs | 4 +- tests/cli_export.rs | 165 +++++++ tests/cli_import.rs | 323 +++++++++++++ tests/cli_status.rs | 8 +- tests/e2e_export_import.rs | 107 +++++ tests/e2e_verify.rs | 77 +++ 46 files changed, 2779 insertions(+), 18 deletions(-) create mode 100644 dev-reports/design/issue-77-design-policy.md create mode 100644 dev-reports/issue/77/issue-review/hypothesis-verification.md create mode 100644 dev-reports/issue/77/issue-review/original-issue.json create mode 100644 dev-reports/issue/77/issue-review/stage1-review-context.json create mode 100644 dev-reports/issue/77/issue-review/stage2-apply-result.json create mode 100644 dev-reports/issue/77/issue-review/stage3-review-context.json create mode 100644 dev-reports/issue/77/issue-review/stage4-apply-result.json create mode 100644 dev-reports/issue/77/issue-review/stage5-review-context.json create mode 100644 dev-reports/issue/77/issue-review/stage6-apply-result.json create mode 100644 dev-reports/issue/77/issue-review/stage7-review-context.json create mode 100644 dev-reports/issue/77/issue-review/stage8-apply-result.json create mode 100644 dev-reports/issue/77/issue-review/summary-report.md create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage1-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage1-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage2-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage2-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage3-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage3-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage4-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage4-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage5-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage6-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage7-review-context.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/stage8-apply-result.json create mode 100644 dev-reports/issue/77/multi-stage-design-review/summary-report.md create mode 100644 dev-reports/issue/77/pm-auto-dev/iteration-1/acceptance-result.json create mode 100644 dev-reports/issue/77/pm-auto-dev/iteration-1/codex-review-result.json create mode 100644 dev-reports/issue/77/pm-auto-dev/iteration-1/refactor-result.json create mode 100644 dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-context.json create mode 100644 dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-result.json create mode 100644 dev-reports/issue/77/work-plan.md create mode 100644 src/cli/export.rs create mode 100644 src/cli/import_index.rs create mode 100644 src/indexer/snapshot.rs create mode 100644 tests/cli_export.rs create mode 100644 tests/cli_import.rs create mode 100644 tests/e2e_export_import.rs create mode 100644 tests/e2e_verify.rs diff --git a/Cargo.lock b/Cargo.lock index 1b15a3d..18a28ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -415,6 +415,7 @@ dependencies = [ "chrono", "clap", "colored", + "flate2", "globset", "lindera", "lindera-tantivy", @@ -426,6 +427,7 @@ dependencies = [ "serde_yaml", "sha2", "tantivy", + "tar", "tempfile", "toml", "tracing", diff --git a/Cargo.toml b/Cargo.toml index 255b339..b08a57b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,8 @@ tree-sitter-python = "0.25" rusqlite = { version = "0.31", features = ["bundled"] } reqwest = { version = "0.12", default-features = false, features = ["blocking", "json", "rustls-tls"] } toml = "0.8" +tar = "0.4" +flate2 = "1" [dev-dependencies] tempfile = "3" diff --git a/dev-reports/design/issue-77-design-policy.md b/dev-reports/design/issue-77-design-policy.md new file mode 100644 index 0000000..226d180 --- /dev/null +++ b/dev-reports/design/issue-77-design-policy.md @@ -0,0 +1,452 @@ +# 設計方針書 - Issue #77: インデックス共有モード + +## 1. 概要 + +CI/CDパイプラインやチーム共有サーバーでインデックスを事前生成し、`export`/`import` サブコマンドでチームメンバーが再利用できる仕組みを提供する。また `status --verify` でインデックスの整合性チェックを行う。 + +## 2. レイヤー構成と責務 + +### 新規モジュール + +| レイヤー | モジュール | 責務 | +|---------|-----------|------| +| **CLI** | `src/cli/export.rs` | エクスポートサブコマンドの実行ロジック（tar.gz圧縮含む） | +| **CLI** | `src/cli/import_index.rs` | インポートサブコマンドの実行ロジック（tar.gz展開・パス検証含む） | +| **Indexer** | `src/indexer/snapshot.rs` | ExportMeta 構造体定義 + export_meta.json の読み書きのみ（SRP） | + +> **設計判断**: snapshot.rs の責務を ExportMeta の型定義とシリアライズ/デシリアライズに限定する。tar.gz 操作は export.rs / import_index.rs の責務とし、パストラバーサル検証は import_index.rs 内の private 関数として実装する。 + +### 変更対象モジュール + +| モジュール | 変更内容 | +|-----------|---------| +| `src/main.rs` | `Commands` enum に `Export` / `Import` バリアント追加 | +| `src/cli/mod.rs` | `pub mod export;` `pub mod import_index;` 追加 | +| `src/cli/status.rs` | `--verify` フラグ対応、`run()` に `verify: bool` 引数追加 | +| `src/indexer/mod.rs` | `pub mod snapshot;` 追加 | +| `Cargo.toml` | `tar`, `flate2` 依存追加 | + +### 変更不要モジュール + +既存の `index`, `search`, `update`, `clean`, `context`, `embed`, `config` コマンドには変更なし。 + +### 定数参照の方針 + +- CLI層 (`src/cli/`): `crate::indexer::commandindex_dir(path)` ヘルパー関数を使用 +- Indexer層 (`src/indexer/`): 同モジュール内の既存定数 `COMMANDINDEX_DIR`, `TANTIVY_DIR` を使用 + +## 3. 技術選定 + +| カテゴリ | 選定技術 | 選定理由 | +|---------|---------|---------| +| tar操作 | `tar` (0.4) | Rust標準的なtarクレート、既にトランジティブ依存として存在 | +| gzip圧縮 | `flate2` (1, default features = miniz_oxide) | 純Rustバックエンド、クロスコンパイル影響なし | +| Git情報取得 | `fn current_git_hash(repo_path: &Path) -> Option` | export.rs 内のユーティリティ関数として分離。テスト時はモック可能 | + +## 4. データ構造設計 + +### 4.1 ExportMeta（エクスポートメタデータ） + +```rust +// src/indexer/snapshot.rs + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ExportMeta { + pub export_format_version: u32, // 初期値: 1、前方互換ポリシー + pub commandindex_version: String, // env!("CARGO_PKG_VERSION") + pub git_commit_hash: Option, // エクスポート時のHEADコミットハッシュ + pub exported_at: DateTime, // エクスポート日時 +} +``` + +> **設計変更**: `index_root` を ExportMeta から削除。エクスポート元の絶対パス漏洩を防止するため。 + +> **バージョン互換性ポリシー**: `export_format_version` は整数インクリメント。import 側は「自身が対応する最大バージョン以下であれば受け入れる」前方互換ポリシー。新フィールド追加時に旧バージョンの import を壊さない。 + +### 4.2 ExportOptions / ImportOptions / ExportResult / ImportResult + +```rust +// src/cli/export.rs + +pub struct ExportOptions { + pub with_embeddings: bool, +} + +pub struct ExportResult { + pub output_path: PathBuf, + pub archive_size: u64, + pub git_commit_hash: Option, +} + +// src/cli/import_index.rs + +pub struct ImportOptions { + pub force: bool, +} + +pub struct ImportResult { + pub imported_files: u64, + pub git_hash_match: bool, + pub warnings: Vec, +} +``` + +> **設計判断**: 既存の IndexSummary/CleanResult パターンに合わせ、ExportResult/ImportResult 構造体で結果を返す。 + +### 4.3 status.rs の verify 対応 + +```rust +// src/cli/status.rs — run() シグネチャ + +pub fn run( + path: &Path, + format: StatusFormat, // format は別引数のまま（CleanOptions パターン準拠） + verify: bool, // 新規追加 + writer: &mut dyn Write, +) -> Result<(), StatusError> +``` + +> **設計判断**: `StatusOptions` 構造体は導入しない。`format` は別引数のまま維持し、`verify: bool` のみ追加する。既存の CleanOptions が `keep_embeddings` のみ（format は含まない）であるパターンに合わせ、オプション構造体の導入は実際の要求が発生してからとする（YAGNI）。既存テストは `verify: false` を追加するだけの最小修正。 + +### 4.4 VerifyResult（整合性チェック結果） + +```rust +// src/cli/status.rs + +#[derive(Debug, Serialize)] +pub struct VerifyResult { + pub state_valid: bool, + pub tantivy_valid: bool, + pub manifest_valid: bool, + pub symbols_valid: bool, + pub issues: Vec, +} + +#[derive(Debug, Serialize)] +pub struct VerifyIssue { + pub component: String, + pub severity: VerifySeverity, + pub message: String, +} + +#[derive(Debug, Serialize)] +pub enum VerifySeverity { + Error, + Warning, +} +``` + +## 5. エラー型設計 + +### 5.1 ExportError + +```rust +// src/cli/export.rs + +#[derive(Debug)] +pub enum ExportError { + NotInitialized, + Io(std::io::Error), + State(StateError), + Manifest(ManifestError), + Serialize(serde_json::Error), + GitError(String), +} + +// 既存パターン準拠: impl fmt::Display, impl std::error::Error (source付き), impl From +impl fmt::Display for ExportError { ... } +impl std::error::Error for ExportError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { ... } } +impl From for ExportError { ... } +impl From for ExportError { ... } +impl From for ExportError { ... } +impl From for ExportError { ... } +``` + +### 5.2 ImportError + +```rust +// src/cli/import_index.rs + +#[derive(Debug)] +pub enum ImportError { + Io(std::io::Error), + ExistingIndex(PathBuf), + PathTraversal(String), + SymlinkDetected(PathBuf), // シンボリックリンク/ハードリンク検出 + InvalidArchive(String), + IncompatibleVersion { expected: u32, found: u32 }, + DecompressionBomb { limit: u64 }, // 展開サイズ上限超過 + State(StateError), + Deserialize(serde_json::Error), +} + +// 同様に impl fmt::Display, impl std::error::Error, impl From +``` + +## 6. 処理フロー + +### 6.1 エクスポートフロー + +``` +commandindex export [--with-embeddings] + 1. .commandindex/ の存在確認（NotInitialized エラー） + 2. IndexState::load() でインデックス状態読み込み + 3. current_git_hash(path) でコミットハッシュ取得（失敗時は None） + 4. ExportMeta を生成（index_root は含めない） + 5. export 前バリデーション: + - tantivy ドキュメント内の path が相対パスであることを確認 + - symbols.db 内の file_path が相対パスであることを確認 + 6. tar::Builder + flate2::GzEncoder でストリーミング圧縮 + - export_meta.json を最初に追加 + - state.json を追加（index_root を placeholder に置換してからパック） + - manifest.json, symbols.db を追加 + - tantivy/ ディレクトリを再帰的に追加 + - --with-embeddings 時のみ embeddings.db を追加 + - config.local.toml は常に除外 + 7. ExportResult を返す（出力パス、サイズ、git hash） +``` + +### 6.2 インポートフロー + +``` +commandindex import [--force] + 1. アーカイブファイルの存在確認 + 2. .commandindex/ の既存チェック + - 存在する場合: --force なしならエラー、ありなら警告後に削除 + 3. flate2::GzDecoder + tar::Archive でストリーミング展開 + 各エントリに対して: + a. パストラバーサルチェック（絶対パス拒否、.. 拒否） + b. シンボリックリンク/ハードリンク拒否（entry_type チェック） + c. 累積展開サイズチェック（上限: 1GB、エントリ数上限: 10000） + d. ファイルパーミッション固定（0o644/0o755） + - 不正検出時は即座にエラー、展開済みファイルをクリーンアップ + 4. export_meta.json を読み込み + - export_format_version の互換性チェック（前方互換ポリシー） + - commandindex_version の長さバリデーション + 5. state.json の index_root をインポート先の絶対パスに書き換え + 6. import 後バリデーション: + - manifest.json 内の各 FileEntry.path が相対パスであることを確認 + - symbols.db 内のパスが相対パスであることを確認 + 7. git rev-parse HEAD で現在のコミットハッシュ取得 + - export_meta の git_commit_hash と比較 + - 不一致時に警告メッセージ表示 + 8. tantivy インデックスのオープン確認 + 9. ImportResult を返す +``` + +### 6.3 整合性チェックフロー + +``` +commandindex status --verify + 1. .commandindex/ の存在確認 + 2. state.json の読み込みと schema_version チェック + 3. tantivy/ ディレクトリの存在確認 + 4. tantivy::Index::open_in_dir() でオープン可能性チェック + 5. manifest.json の読み込みと各ファイルの存在確認 + 6. symbols.db のオープンとスキーマバージョン確認 + 7. VerifyResult を構築して出力 +``` + +## 7. セキュリティ設計 + +| 脅威 | 対策 | 実装方針 | +|------|------|---------| +| パストラバーサル（ファイルパス） | 文字列レベルの検証 | 絶対パス拒否 + `..` コンポーネント拒否 + join 後の components() 再チェック。**canonicalize() は使わない**（展開前にファイルが存在しないため） | +| パストラバーサル（シンボリックリンク） | entry_type チェック | `Symlink` / `Link` エントリは即座に拒否。展開済み親ディレクトリの `symlink_metadata()` 確認。clean.rs の SymlinkDetected パターン踏襲 | +| パストラバーサル（ハードリンク） | entry_type チェック | `Link`（ハードリンク）エントリも拒否 | +| 機密情報漏洩（ファイル） | config.local.toml 除外 | エクスポート時に明示的スキップ | +| 機密情報漏洩（パス） | index_root サニタイズ | state.json の index_root を placeholder に置換してパック。export_meta.json には index_root を含めない | +| 圧縮爆弾 | サイズ/エントリ数上限 | 累積展開バイト数上限 (1GB)、個別エントリサイズ上限、エントリ数上限 (10000) | +| 悪意あるメタデータ | デシリアライズ検証 | `#[serde(deny_unknown_fields)]`、文字列長上限チェック | +| 権限昇格 | パーミッション固定 | 展開ファイルのパーミッションを 0o644/0o755 に固定、tarエントリの権限情報は無視 | + +### パストラバーサル検証の実装方針 + +```rust +fn validate_entry_path(entry_path: &Path, target_dir: &Path) -> Result { + // 1. 絶対パスの拒否 + if entry_path.is_absolute() { + return Err(ImportError::PathTraversal(format!("absolute path: {:?}", entry_path))); + } + // 2. ".." コンポーネントの拒否 + for component in entry_path.components() { + if matches!(component, std::path::Component::ParentDir) { + return Err(ImportError::PathTraversal(format!("parent dir: {:?}", entry_path))); + } + } + // 3. 展開先パスの構築 + let full_path = target_dir.join(entry_path); + // 4. 正規化後の prefix チェック（canonicalize は使わない） + // components() で再構築して target_dir で始まることを確認 + Ok(full_path) +} + +fn validate_entry_type(entry: &tar::Entry) -> Result<(), ImportError> { + match entry.header().entry_type() { + tar::EntryType::Symlink | tar::EntryType::Link => { + Err(ImportError::SymlinkDetected(entry.path()?.to_path_buf())) + } + _ => Ok(()), + } +} +``` + +## 8. CLIインターフェース設計 + +### main.rs への追加 + +```rust +// Commands enum に追加（doc comment は英語で統一） +/// Export index as portable tar.gz archive +Export { + /// Output file path (.tar.gz) + output: PathBuf, + /// Include embedding database + #[arg(long)] + with_embeddings: bool, +}, +/// Import index from tar.gz archive +Import { + /// Input archive file path (.tar.gz) + input: PathBuf, + /// Overwrite existing index + #[arg(long)] + force: bool, +}, + +// Status バリアントに verify 追加 +Status { + #[arg(long, default_value = ".")] + path: PathBuf, + #[arg(long, value_enum, default_value_t = StatusFormat::Human)] + format: StatusFormat, + /// Verify index integrity + #[arg(long)] + verify: bool, +}, +``` + +### run() 関数シグネチャ（既存パターン準拠） + +```rust +// export.rs — 第1引数は base path（既存パターン） +pub fn run(path: &Path, output: &Path, options: &ExportOptions) -> Result + +// import_index.rs — 第1引数は base path、第2引数は archive path +pub fn run(path: &Path, archive: &Path, options: &ImportOptions) -> Result + +// status.rs — format は別引数のまま、verify を追加 +pub fn run(path: &Path, format: StatusFormat, verify: bool, writer: &mut dyn Write) -> Result<(), StatusError> +``` + +## 9. アーカイブ内部構造 + +``` +index-snapshot.tar.gz +├── export_meta.json # エクスポートメタデータ（最初のエントリ） +├── state.json # インデックス状態（index_root はサニタイズ済み） +├── manifest.json # ファイルマニフェスト（相対パス） +├── symbols.db # シンボルデータベース（相対パス） +├── tantivy/ # tantivy インデックスディレクトリ +│ ├── meta.json +│ ├── .managed.json +│ └── *.{del,fast,fieldnorm,idx,pos,store,term} +└── embeddings.db # (--with-embeddings 時のみ) +``` + +**注意**: アーカイブ内のパスは `.commandindex/` プレフィックスなしのフラットな構造。インポート時に `.commandindex/` ディレクトリ内に展開される。commandindex.toml（リポジトリルート）はエクスポート対象外。 + +## 10. 設計判断とトレードオフ + +| 判断 | 選択 | 代替案 | 理由 | +|------|------|--------|------| +| CLI設計 | 独立サブコマンド (`export`/`import`) | `index --export`/`index --import` | 単一責任パターンとの整合性、clap排他制御の複雑化回避 | +| メタデータ | 別ファイル (`export_meta.json`) | `state.json` にフィールド追加 | state.json の後方互換性維持、エクスポート固有情報の分離 | +| パス検証 | 手動エントリ展開 | `Archive::unpack()` | セキュリティ：パストラバーサル防止を確実にするため | +| パス検証方式 | 文字列レベル + components() | `canonicalize()` | 展開前にファイルが存在しないため canonicalize() は使えない | +| embeddings.db | デフォルト除外 | デフォルト含む | モデル依存データであり、異なる環境では意味をなさない可能性 | +| アーカイブパス | フラットパス（プレフィックスなし） | `.commandindex/` プレフィックス付き | インポート時の柔軟性、展開先ディレクトリの明示的制御 | +| status拡張 | `verify: bool` 引数追加 | StatusOptions構造体 | YAGNI — CleanOptionsパターン準拠、構造体は実際に複数オプションが必要になってから | +| import モジュール名 | `import_index.rs` | `import.rs` | 既存モジュールは単一単語だが、import はキーワード衝突の可能性回避 | +| snapshot.rs の責務 | ExportMeta の型+読み書きのみ | tar操作も含む | SRP — tar操作は export.rs/import_index.rs の責務 | +| index_root 漏洩防止 | ExportMeta に含めない + state.json サニタイズ | そのまま含める | 絶対パスによるインフラ情報漏洩を防止 | +| 圧縮爆弾対策 | サイズ/エントリ数上限 | ストリーミングのみ | ストリーミングはメモリ節約のみ、ディスク枯渇は防げない | +| git hash 取得 | 分離関数 | snapshot.rs 内に含める | DIP — テスタビリティ向上、外部コマンド依存の分離 | + +## 11. 影響範囲 + +### 直接影響 + +| ファイル | 変更種別 | 影響度 | +|---------|---------|--------| +| `src/main.rs` | enum バリアント追加 + match 分岐追加 | 低 | +| `src/cli/mod.rs` | モジュール宣言追加 | 低 | +| `src/cli/status.rs` | run() に verify 引数追加、verify ロジック実装 | 中 | +| `src/indexer/mod.rs` | モジュール宣言追加 | 低 | +| `Cargo.toml` | 依存追加 | 低 | +| `tests/cli_args.rs` | help テストに export/import 検証追加 | 低 | +| `tests/cli_status.rs` | run() 呼び出しに verify: false 追加（4箇所） | 低 | + +### 新規ファイル + +| ファイル | 内容 | +|---------|------| +| `src/cli/export.rs` | エクスポートサブコマンド + ExportResult/ExportError | +| `src/cli/import_index.rs` | インポートサブコマンド + ImportResult/ImportError | +| `src/indexer/snapshot.rs` | ExportMeta 構造体 + 読み書き | +| `tests/cli_export.rs` | エクスポート統合テスト | +| `tests/cli_import.rs` | インポート統合テスト | +| `tests/e2e_export_import.rs` | export → import → search E2Eテスト | + +### 間接影響 + +- 既存の `index`, `search`, `update`, `clean`, `embed`, `context`, `config` コマンド: **影響なし** +- CI/CDパイプライン: **変更不要**（tar/flate2 は純Rust、クロスコンパイル問題なし） +- export 成果物の出力先: `.commandindex/` 外（ユーザー指定パス）のため clean コマンドに影響なし + +### import 後の既存コマンドとの整合性 + +- `update`: import 後に実行可能。index_root 書き換え済みのため差分検出が正常動作する（統合テストで検証必須） +- `clean`: import したインデックスも通常通り削除可能 +- `search`: import 後の検索が正常動作する（E2Eテストで検証必須） + +## 12. テスト戦略 + +### 新規テスト + +| テストファイル | テスト内容 | 種別 | +|--------------|-----------|------| +| `tests/cli_export.rs` | export 基本動作、NotInitialized エラー | 統合 | +| | config.local.toml がエクスポートに含まれないことの検証 | セキュリティ | +| | embeddings.db のデフォルト除外と --with-embeddings | 統合 | +| `tests/cli_import.rs` | import 基本動作 | 統合 | +| | 既存インデックスありで --force なしのエラー | 統合 | +| | 既存インデックスありで --force のインポート | 統合 | +| | パストラバーサル検出テスト（`../`, 絶対パス） | セキュリティ | +| | シンボリックリンクエントリ拒否テスト | セキュリティ | +| | ハードリンクエントリ拒否テスト | セキュリティ | +| | 圧縮爆弾検出テスト（サイズ上限超過） | セキュリティ | +| | export_format_version 不一致時のエラー | 統合 | +| | コミットハッシュ不一致時の警告 | 統合 | +| `tests/e2e_export_import.rs` | export → import → search の E2E フロー | E2E | +| | import 後に update が正常動作するか | E2E | +| | import 後に tantivy インデックスがオープンできるか | E2E | +| `tests/e2e_verify.rs` | 正常インデックスの verify パス | E2E | +| | 破損インデックスの verify エラー検出 | E2E | + +### 既存テスト修正 + +| テストファイル | 修正内容 | +|--------------|---------| +| `tests/cli_args.rs` | `help_flag_shows_usage` に export/import の検証追加 | +| `tests/cli_status.rs` | `run()` 呼び出しに `verify: false` 追加（4箇所） | + +## 13. 品質基準 + +| チェック項目 | コマンド | 基準 | +|-------------|----------|------| +| ビルド | `cargo build` | エラー0件 | +| Clippy | `cargo clippy --all-targets -- -D warnings` | 警告0件 | +| テスト | `cargo test --all` | 全テストパス | +| フォーマット | `cargo fmt --all -- --check` | 差分なし | diff --git a/dev-reports/issue/77/issue-review/hypothesis-verification.md b/dev-reports/issue/77/issue-review/hypothesis-verification.md new file mode 100644 index 0000000..95f30c3 --- /dev/null +++ b/dev-reports/issue/77/issue-review/hypothesis-verification.md @@ -0,0 +1,6 @@ +# 仮説検証レポート - Issue #77 + +## 結果: スキップ + +Issue #77 は新機能提案（インデックス共有モード）であり、仮説・原因分析の記載はありません。 +仮説検証フェーズをスキップします。 diff --git a/dev-reports/issue/77/issue-review/original-issue.json b/dev-reports/issue/77/issue-review/original-issue.json new file mode 100644 index 0000000..d698df7 --- /dev/null +++ b/dev-reports/issue/77/issue-review/original-issue.json @@ -0,0 +1 @@ +{"body":"## 概要\n\nCI/CDパイプラインやチーム共有サーバーでインデックスを事前生成し、チームメンバーが再利用できる仕組みを提供する。\n\n## 背景・動機\n\n大規模リポジトリでは `commandindex index` の初回実行に時間がかかる。CIでインデックスを事前生成し、チームメンバーがダウンロードして利用できると、オンボーディングや日常利用が効率化する。\n\n## 提案する解決策\n\n### インデックスのエクスポート/インポート\n\n```bash\n# インデックスをtar.gzにエクスポート\ncommandindex index --export index-snapshot.tar.gz\n\n# エクスポートされたインデックスをインポート\ncommandindex index --import index-snapshot.tar.gz\n\n# インデックスの整合性チェック\ncommandindex status --verify\n```\n\n### CI連携例\n\n```yaml\n# GitHub Actions\n- name: Build CommandIndex\n run: |\n commandindex index\n commandindex index --export /tmp/commandindex-snapshot.tar.gz\n\n- name: Upload artifact\n uses: actions/upload-artifact@v4\n with:\n name: commandindex-snapshot\n path: /tmp/commandindex-snapshot.tar.gz\n```\n\n### エクスポートフォーマット\n\n- `.commandindex/` ディレクトリをtar.gzで圧縮\n- `state.json` にエクスポート時のメタデータ（コミットハッシュ、タイムスタンプ）を追加\n- インポート時にコミットハッシュの一致を確認（不一致の場合は警告）\n\n### --verify オプション\n\n- tantivy インデックスの整合性チェック\n- `manifest.json` とファイルシステムの一致確認\n- `symbols.db` のスキーマバージョン確認\n\n## 受け入れ基準\n\n- [ ] `--export` でインデックスをtar.gzにエクスポートできる\n- [ ] `--import` でインデックスをインポートできる\n- [ ] インポート後の検索が正常に動作する\n- [ ] `status --verify` でインデックスの整合性チェックができる\n- [ ] コミットハッシュ不一致時に警告メッセージが表示される\n- [ ] cargo test / clippy / fmt 全パス\n\n## 依存 Issue\n\n- #76 チーム共有設定ファイル","title":"[Feature] インデックス共有モード（--shared / CI連携）"} diff --git a/dev-reports/issue/77/issue-review/stage1-review-context.json b/dev-reports/issue/77/issue-review/stage1-review-context.json new file mode 100644 index 0000000..6da058d --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage1-review-context.json @@ -0,0 +1,81 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "CLIインターフェース設計がサブコマンド構成と不整合", + "detail": "Issueでは `commandindex index --export` / `commandindex index --import` と既存の `index` サブコマンドにフラグを追加する形を提案しているが、既存の `Index` サブコマンドは `--path` と `--with-embedding` のみを持つインデックス構築用コマンドであり、エクスポート/インポートはインデックス構築とは本質的に異なる操作である。--export/--import を index のフラグにすると、--path や --with-embedding との排他制御が必要になり、clap定義が複雑化する。", + "suggestion": "エクスポート/インポートは独立したサブコマンド `commandindex export ` / `commandindex import ` として設計するか、`commandindex index export` / `commandindex index import` のようにネストしたサブコマンドにする方が既存のCLI設計パターンと整合する。" + }, + { + "id": "M2", + "title": "state.jsonにコミットハッシュのフィールドが存在しない", + "detail": "Issueでは「state.json にエクスポート時のメタデータ（コミットハッシュ、タイムスタンプ）を追加」と記載しているが、現在の IndexState 構造体には git_commit_hash フィールドが存在しない。追加時の後方互換性への影響を考慮する必要がある。", + "suggestion": "IndexState にオプショナルフィールド `#[serde(default)] pub git_commit_hash: Option` を追加するか、エクスポート専用のメタデータファイル（export_meta.json）をアーカイブ内に別途含める設計を明記すべき。" + }, + { + "id": "M3", + "title": "セキュリティ: tar展開時のパストラバーサル対策が未記載", + "detail": "信頼できないソースからのtar.gzアーカイブをインポートする場合、パストラバーサル攻撃が成立する可能性がある。受け入れ基準にセキュリティ要件が含まれていない。", + "suggestion": "受け入れ基準に「インポート時、アーカイブ内の全エントリが展開先ディレクトリ内に収まることを検証し、パストラバーサルを検出した場合はエラーで中断する」を追加する。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "既存インデックスがある場合のインポート動作が未定義", + "detail": "インポート先に既に .commandindex/ が存在する場合の挙動が未記載。", + "suggestion": "「既存インデックスがある場合は警告を表示し、--force フラグがなければ中断する」を追加する。" + }, + { + "id": "S2", + "title": "index_root パスの不整合問題が未考慮", + "detail": "state.json の index_root にはインデックス作成時の絶対パスが保存されるが、インポート先では異なるパスになる。", + "suggestion": "インポート時に index_root をインポート先のパスに自動書き換える処理を仕様に含めるべき。" + }, + { + "id": "S3", + "title": "symbols.db と embeddings.db のポータビリティ考慮不足", + "detail": "SQLiteデータベースファイル内のファイルパスが相対パスか絶対パスかによってポータビリティが変わる。", + "suggestion": "エクスポート対象ファイル一覧と各ファイル内のパスが相対パスであることの前提条件を明記する。" + }, + { + "id": "S4", + "title": "--verify の検証範囲が曖昧", + "detail": "「manifest.json とファイルシステムの一致確認」が具体的に何を指すか不明確。", + "suggestion": "各チェック項目を具体的に定義する。例: (1) state.json の schema_version 確認、(2) tantivy ディレクトリの存在と読み取り可能性、(3) manifest.json に記載されたファイルの存在確認、(4) symbols.db の schema_version 確認。" + }, + { + "id": "S5", + "title": "tar.gz の依存クレート選定が未検討", + "detail": "Cargo.toml に tar/flate2 等の圧縮関連クレートが含まれていない。", + "suggestion": "使用予定のクレート（tar + flate2）を明記する。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "エクスポートファイルのバージョニング", + "detail": "将来のスキーマ変更に備え、エクスポートファイル自体にフォーマットバージョンを含めると便利。", + "suggestion": "アーカイブ内に export_meta.json を含め、export_format_version, commandindex_version 等を記録する。" + }, + { + "id": "N2", + "title": "差分インポートとの連携", + "detail": "インポート後に commandindex update で差分更新できるかどうかが未記載。", + "suggestion": "インポート後に差分更新が正常動作することを確認する旨を追記する。" + }, + { + "id": "N3", + "title": "エクスポート/インポートの進捗表示", + "detail": "大規模リポジトリでは圧縮・展開に時間がかかる可能性がある。", + "suggestion": "初期実装ではサイズ表示のみとし、将来的にプログレスバーを追加する。" + }, + { + "id": "N4", + "title": "config.local.toml の除外", + "detail": "エクスポート時に個人設定やAPIキーを含む可能性のある config.local.toml を含めるべきでない。", + "suggestion": "エクスポート対象から config.local.toml を明示的に除外する旨を記載する。" + } + ], + "summary": "Issue #77 は大規模リポジトリでのオンボーディング効率化という明確な価値を持つ良いFeatureリクエストである。ただし、(1) CLIインターフェース設計の不整合、(2) state.jsonへのコミットハッシュフィールド追加方針、(3) tar展開時のセキュリティ対策の3点が必須修正。加えて、index_rootの環境間差異、既存インデックスの上書きポリシー、--verifyの検証範囲、依存クレートの選定等の明確化が推奨される。" +} diff --git a/dev-reports/issue/77/issue-review/stage2-apply-result.json b/dev-reports/issue/77/issue-review/stage2-apply-result.json new file mode 100644 index 0000000..5cac6f2 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage2-apply-result.json @@ -0,0 +1,20 @@ +{ + "stage": 2, + "applied_fixes": [ + {"id": "M1", "action": "CLIインターフェースを独立サブコマンド (export/import) に変更"}, + {"id": "M2", "action": "export_meta.json をアーカイブ内に含める設計に変更、state.jsonへの直接変更を回避"}, + {"id": "M3", "action": "セキュリティ要件セクションを追加、受け入れ基準にパストラバーサル検出を追加"}, + {"id": "S1", "action": "既存インデックスがある場合の動作セクションを追加、--force フラグ追加"}, + {"id": "S2", "action": "インポート時の index_root 自動書き換えを仕様に追加"}, + {"id": "S3", "action": "エクスポート対象ファイル一覧テーブルを追加、パス形式を明記"}, + {"id": "S4", "action": "--verify の各チェック項目を具体的に定義"}, + {"id": "S5", "action": "依存クレート（tar + flate2）セクションを追加"}, + {"id": "N1", "action": "export_meta.json にフォーマットバージョンを含める設計に採用"}, + {"id": "N4", "action": "config.local.toml の除外を明記"} + ], + "deferred": [ + {"id": "N2", "reason": "差分インポートは将来のエンハンスメントとして別Issue化が適切"}, + {"id": "N3", "reason": "進捗表示は初期実装のスコープ外"} + ], + "issue_updated": true +} diff --git a/dev-reports/issue/77/issue-review/stage3-review-context.json b/dev-reports/issue/77/issue-review/stage3-review-context.json new file mode 100644 index 0000000..3f6adf7 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage3-review-context.json @@ -0,0 +1,81 @@ +{ + "must_fix": [ + { + "id": "M1", + "title": "tantivy インデックスの export/import 後の整合性検証が必要", + "detail": "tantivy の meta.json やセグメントファイルが export/import 後に正常にオープンできるかの検証が必須。", + "suggestion": "統合テストを追加し、export → import 後に tantivy インデックスが開けること・検索が動作することを検証する。" + }, + { + "id": "M2", + "title": "symbols.db / embeddings.db 内のパス整合性", + "detail": "SQLite 内のファイルパスは相対パスで格納されているが、index_root 書き換えのみで全検索ロジックが正常動作するか検証が必要。", + "suggestion": "index_root の書き換えのみで動作する設計方針をドキュメント化し、統合テストで検証する。" + }, + { + "id": "M3", + "title": "パストラバーサル防止の実装", + "detail": "tar クレートの Archive::unpack はデフォルトでパストラバーサルを防止しない。エントリごとのパス検証が必要。", + "suggestion": "エントリごとに path().starts_with(target_dir) をチェックし、悪意あるアーカイブのテストケースも追加する。" + }, + { + "id": "M4", + "title": "cli_args.rs テストへの新サブコマンド検証追加", + "detail": "help_flag_shows_usage テストに export/import の存在検証を追加すべき。", + "suggestion": "テストに .stdout(predicate::str::contains(\"export\")) と import の検証を追加。" + } + ], + "should_fix": [ + { + "id": "S1", + "title": "status --verify と既存 StatusFormat への影響", + "detail": "run() の関数シグネチャ変更で既存テスト (cli_status.rs 11個) の修正が発生する可能性。", + "suggestion": "StatusOptions 構造体を導入し後方互換性を保つ設計を推奨。" + }, + { + "id": "S2", + "title": "export_meta.json のバージョン互換性チェック", + "detail": "将来のバージョンアップ時に古いエクスポートをインポートできるよう設計が必要。", + "suggestion": "export_schema_version フィールドを設け、import 時にバージョン互換性チェックを実施。" + }, + { + "id": "S3", + "title": "cli/mod.rs へのモジュール宣言追加", + "detail": "export.rs / import.rs を追加する場合、mod.rs への宣言追加が必要。", + "suggestion": "アルファベット順で追加。" + }, + { + "id": "S4", + "title": "大規模インデックスのメモリ使用量", + "detail": "tar/gzip のストリーミング API を使用してメモリ使用量を抑制すべき。", + "suggestion": "ストリーミング圧縮/展開を使用する設計にする。" + }, + { + "id": "S5", + "title": "--force フラグなしでの既存インデックス保護", + "detail": "import 前に既存インデックスの存在チェックとエラーメッセージが必要。", + "suggestion": "IndexState::exists() でチェックし、明確なエラーメッセージで中断。" + } + ], + "nice_to_have": [ + { + "id": "N1", + "title": "tar/flate2 は既にトランジティブ依存として存在", + "detail": "lindera-dictionary の build-dependencies として既に依存ツリーに存在。バイナリサイズへの影響は最小限。", + "suggestion": "Cargo.toml に tar = \"0.4\" と flate2 = \"1\" を追加。" + }, + { + "id": "N2", + "title": "クロスコンパイルへの影響なし", + "detail": "tar/flate2 は純 Rust クレートでありC依存がないため問題なし。", + "suggestion": "特に対応不要。" + }, + { + "id": "N3", + "title": "embeddings.db の export 可否オプション", + "detail": "embeddings.db はモデル依存データであり、異なる環境では意味をなさない可能性がある。", + "suggestion": "デフォルトでは embeddings.db を含めず、--with-embeddings フラグで明示的に含める設計を推奨。" + } + ], + "summary": "新規サブコマンド追加が中心で既存機能への影響は低い。最も注意が必要なのは tantivy インデックスの整合性、パストラバーサル防止、index_root 書き換えの3点。依存クレートは既にトランジティブに存在しており、CI/CDへの影響は最小限。" +} diff --git a/dev-reports/issue/77/issue-review/stage4-apply-result.json b/dev-reports/issue/77/issue-review/stage4-apply-result.json new file mode 100644 index 0000000..3501fc1 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage4-apply-result.json @@ -0,0 +1,20 @@ +{ + "stage": 4, + "applied_fixes": [ + {"id": "M1", "action": "tantivy インデックスの export/import 後の整合性検証を受け入れ基準に追加"}, + {"id": "M2", "action": "SQLite内のパスが相対パスであることの注記を追加"}, + {"id": "M3", "action": "セキュリティ要件にエントリごとのパス検証方法と悪意あるアーカイブのテストケースを追記"}, + {"id": "M4", "action": "実装上の注意点に cli_args.rs テスト更新を追記"}, + {"id": "S1", "action": "StatusOptions 構造体導入の方針を実装注意点に追記"}, + {"id": "S2", "action": "export_format_version の互換性チェックを受け入れ基準に追加"}, + {"id": "S3", "action": "cli/mod.rs へのモジュール追加を実装注意点に追記"}, + {"id": "S4", "action": "ストリーミングAPIの使用をエクスポートフォーマットセクションに追記"}, + {"id": "S5", "action": "明確なエラーメッセージ表示を既存インデックス保護セクションに追記"}, + {"id": "N3", "action": "embeddings.db をデフォルト除外、--with-embeddings フラグを追加"} + ], + "deferred": [ + {"id": "N1", "reason": "tar/flate2 のトランジティブ依存情報は依存クレートセクションに注記追加"}, + {"id": "N2", "reason": "クロスコンパイル影響なしの注記を依存クレートセクションに追加"} + ], + "issue_updated": true +} diff --git a/dev-reports/issue/77/issue-review/stage5-review-context.json b/dev-reports/issue/77/issue-review/stage5-review-context.json new file mode 100644 index 0000000..a8a3520 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage5-review-context.json @@ -0,0 +1,9 @@ +{ + "stage": 5, + "status": "skipped", + "reason": "Codex (commandmatedev) がレスポンスを生成しなかったため、Stage 5 をスキップ。Stage 1-4 の Claude opus レビューで十分な品質改善が達成済み。", + "must_fix": [], + "should_fix": [], + "nice_to_have": [], + "summary": "Codex 通常レビュー（2回目）はスキップ。1回目レビュー (Stage 1-4) で Must Fix 7件を全て反映済み。" +} diff --git a/dev-reports/issue/77/issue-review/stage6-apply-result.json b/dev-reports/issue/77/issue-review/stage6-apply-result.json new file mode 100644 index 0000000..e65aa64 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage6-apply-result.json @@ -0,0 +1,5 @@ +{ + "stage": 6, + "status": "skipped", + "reason": "Stage 5 がスキップされたため、Stage 6 もスキップ。" +} diff --git a/dev-reports/issue/77/issue-review/stage7-review-context.json b/dev-reports/issue/77/issue-review/stage7-review-context.json new file mode 100644 index 0000000..c1584e2 --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage7-review-context.json @@ -0,0 +1,9 @@ +{ + "stage": 7, + "status": "skipped", + "reason": "Codex がレスポンスを生成しなかったため、Stage 7 をスキップ。Stage 3-4 の Claude opus 影響範囲レビューで十分なカバレッジを達成済み。", + "must_fix": [], + "should_fix": [], + "nice_to_have": [], + "summary": "Codex 影響範囲レビュー（2回目）はスキップ。1回目影響範囲レビュー (Stage 3-4) の指摘事項は全て反映済み。" +} diff --git a/dev-reports/issue/77/issue-review/stage8-apply-result.json b/dev-reports/issue/77/issue-review/stage8-apply-result.json new file mode 100644 index 0000000..da3dd9d --- /dev/null +++ b/dev-reports/issue/77/issue-review/stage8-apply-result.json @@ -0,0 +1,5 @@ +{ + "stage": 8, + "status": "skipped", + "reason": "Stage 7 がスキップされたため、Stage 8 もスキップ。" +} diff --git a/dev-reports/issue/77/issue-review/summary-report.md b/dev-reports/issue/77/issue-review/summary-report.md new file mode 100644 index 0000000..b07b84f --- /dev/null +++ b/dev-reports/issue/77/issue-review/summary-report.md @@ -0,0 +1,52 @@ +# Issue #77 マルチステージレビュー サマリーレポート + +## 概要 +Issue #77「[Feature] インデックス共有モード（--shared / CI連携）」に対してマルチステージレビューを実施し、Issueの品質を大幅に改善した。 + +## 実施ステージ + +| Stage | 種別 | 実行エージェント | 結果 | +|-------|------|----------------|------| +| 0.5 | 仮説検証 | - | スキップ（新機能提案のため仮説なし） | +| 1 | 通常レビュー（1回目） | Claude opus | Must Fix: 3件, Should Fix: 5件, Nice to Have: 4件 | +| 2 | 指摘反映（1回目） | Claude sonnet | 10件反映、2件延期 | +| 3 | 影響範囲レビュー（1回目） | Claude opus | Must Fix: 4件, Should Fix: 5件, Nice to Have: 5件 | +| 4 | 指摘反映（1回目） | Claude sonnet | 10件反映、2件情報追記 | +| 5 | 通常レビュー（2回目） | Codex | スキップ（Codex応答なし） | +| 6 | 指摘反映（2回目） | - | スキップ | +| 7 | 影響範囲レビュー（2回目） | Codex | スキップ（Codex応答なし） | +| 8 | 指摘反映（2回目） | - | スキップ | + +## 主要な改善内容 + +### CLIインターフェース設計 (Must Fix) +- **変更前**: `commandindex index --export/--import` （既存サブコマンドへのフラグ追加） +- **変更後**: `commandindex export/import` （独立サブコマンド） +- **理由**: 既存のCLI設計パターン（単一責任）との整合性 + +### メタデータ管理 (Must Fix) +- **変更前**: state.json にコミットハッシュを直接追加 +- **変更後**: `export_meta.json` を別途アーカイブ内に含める設計 +- **理由**: 後方互換性の維持、エクスポート固有情報の分離 + +### セキュリティ (Must Fix) +- パストラバーサル防止の具体的実装方針を追加 +- 悪意あるアーカイブに対するテストケースの追加を明記 +- `config.local.toml` のエクスポート除外を明記 + +### 影響範囲の明確化 +- tantivy インデックスの export/import 後の整合性検証 +- `index_root` の自動書き換えによるポータビリティ確保 +- `embeddings.db` のデフォルト除外（`--with-embeddings` フラグ） +- `--verify` オプションの具体的チェック項目定義 +- 既存インデックスがある場合の `--force` フラグ動作 + +### 依存関係・CI影響 +- `tar` + `flate2` クレートは既にトランジティブ依存として存在 +- 純Rustクレートでありクロスコンパイル影響なし +- CI/CDパイプライン変更不要 + +## Issue 更新状況 +- GitHub Issue #77 は2回更新済み（Stage 2, Stage 4） +- 受け入れ基準: 6項目 → 14項目に拡充 +- 新規セクション追加: セキュリティ要件、エクスポート対象ファイル一覧、既存インデックスの動作、実装上の注意点、依存クレート diff --git a/dev-reports/issue/77/multi-stage-design-review/stage1-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage1-apply-result.json new file mode 100644 index 0000000..654945a --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage1-apply-result.json @@ -0,0 +1 @@ +{"stage": 1, "applied": ["M1: verify を StatusOptions 構造体ではなく run() 引数に変更 (YAGNI)", "M2: snapshot.rs の責務を ExportMeta 定義+読み書きのみに限定 (SRP)", "S2: export_format_version の前方互換ポリシーを明記", "S3: git hash 取得を分離関数に (DIP)"]} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage1-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage1-review-context.json new file mode 100644 index 0000000..755de80 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage1-review-context.json @@ -0,0 +1,21 @@ +{ + "stage": 1, + "type": "設計原則レビュー (SOLID/KISS/YAGNI/DRY)", + "must_fix": [ + {"id": "M1", "title": "VerifyResult/StatusOptions は YAGNI — verify は import 内部検証にとどめるべき"}, + {"id": "M2", "title": "snapshot.rs の SRP 違反 — 責務を ExportMeta 定義+読み書きに限定すべき"} + ], + "should_fix": [ + {"id": "S1", "title": "ExportError/ImportError の共通バリアント DRY 改善"}, + {"id": "S2", "title": "export_format_version の互換性ポリシー未定義 (OCP)"}, + {"id": "S3", "title": "git_commit_hash 取得の DIP — 関数として分離すべき"} + ], + "nice_to_have": [ + {"id": "N1", "title": "独立サブコマンド設計は KISS 適合（維持）"}, + {"id": "N2", "title": "export_meta.json 分離は SRP 適合（維持）"}, + {"id": "N3", "title": "フラットアーカイブ構造の将来拡張時注意"}, + {"id": "N4", "title": "import_index.rs 命名は適切（維持）"}, + {"id": "N5", "title": "手動展開はセキュリティ上正当（維持）"} + ], + "must_fix_count": 2 +} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage2-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage2-apply-result.json new file mode 100644 index 0000000..3c9101a --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage2-apply-result.json @@ -0,0 +1 @@ +{"stage": 2, "applied": ["M1: 定数参照の方針を明記（CLI層は commandindex_dir() ヘルパー使用）", "M2: StatusOptions ではなく verify: bool 引数追加に変更（CleanOptions パターン準拠）", "S1: エラー型に std::error::Error + From 実装の明記", "S2: Commands enum の doc comment を英語に統一", "S4: run() 戻り値を ExportResult/ImportResult に統一"]} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage2-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage2-review-context.json new file mode 100644 index 0000000..beffb09 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage2-review-context.json @@ -0,0 +1,20 @@ +{ + "stage": 2, + "type": "整合性レビュー", + "must_fix": [ + {"id": "M1", "title": "COMMANDINDEX_DIR/INDEX_DIR_NAME 定数の重複 — 参照先統一が必要"}, + {"id": "M2", "title": "Status run() シグネチャ変更 — StatusOptions に format を含めるか CleanOptions パターンに合わせるか"} + ], + "should_fix": [ + {"id": "S1", "title": "ExportError/ImportError に std::error::Error + From 実装の明記"}, + {"id": "S2", "title": "Commands enum の doc comment を英語に統一"}, + {"id": "S3", "title": "import_index.rs の命名根拠明記（import は Rust 2024 で予約語でない）"}, + {"id": "S4", "title": "run() 戻り値を ExportResult/ImportResult 構造体に統一"} + ], + "nice_to_have": [ + {"id": "N1", "title": "snapshot.rs の indexer 配下配置は合理的"}, + {"id": "N2", "title": "テストファイル命名を cli_export.rs/cli_import.rs に"}, + {"id": "N3", "title": "verify の Status 同居は YAGNI 的に妥当"} + ], + "must_fix_count": 2 +} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage3-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage3-apply-result.json new file mode 100644 index 0000000..7f60f31 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage3-apply-result.json @@ -0,0 +1 @@ +{"stage": 3, "applied": ["M1: status.rs run() は verify: bool 引数追加（テスト修正は4箇所のみ）", "M2: import 後の update 整合性を統合テストで検証する旨を追記", "M3: export 前に tantivy 内パスが相対パスであることのバリデーション追加", "M4: export 前に symbols.db 内パスが相対パスであることのバリデーション追加", "S5: export 成果物は .commandindex/ 外に出力するため clean に影響なし"]} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage3-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage3-review-context.json new file mode 100644 index 0000000..4e5b47f --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage3-review-context.json @@ -0,0 +1,24 @@ +{ + "stage": 3, + "type": "影響分析レビュー", + "must_fix": [ + {"id": "M1", "title": "status.rs run() シグネチャ変更による既存5箇所の破壊的変更"}, + {"id": "M2", "title": "import 後の index_root 書き換えと update コマンドの整合性"}, + {"id": "M3", "title": "tantivy ドキュメント内 path が相対パスであることのバリデーション"}, + {"id": "M4", "title": "symbols.db 内パスが相対パスであることの保証"} + ], + "should_fix": [ + {"id": "S1", "title": "cli_args.rs help テストに export/import アサーション追加"}, + {"id": "S2", "title": "flate2 の pure Rust バックエンド確認"}, + {"id": "S3", "title": "embeddings.db のモデル互換性チェック"}, + {"id": "S4", "title": "export 後の clean/index による陳腐化検出"}, + {"id": "S5", "title": "clean コマンドの削除対象に export 成果物が未考慮"} + ], + "nice_to_have": [ + {"id": "N1", "title": "バイナリサイズ before/after 比較"}, + {"id": "N2", "title": "進捗表示の将来対応"}, + {"id": "N3", "title": "SymbolStore スキーマバージョン不整合のハンドリング"}, + {"id": "N4", "title": "snapshot.rs の indexer 配下配置の妥当性"} + ], + "must_fix_count": 4 +} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage4-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage4-apply-result.json new file mode 100644 index 0000000..df7b9db --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage4-apply-result.json @@ -0,0 +1 @@ +{"stage": 4, "applied": ["M1: シンボリックリンク/ハードリンク拒否を validate_entry_type() として追加", "M2: canonicalize 記述を削除、文字列レベル+components()検証に一本化", "M3: 圧縮爆弾対策（サイズ上限1GB, エントリ数上限10000）を追加", "S1: ExportMeta から index_root 削除、state.json はサニタイズしてパック", "S3: ExportMeta に #[serde(deny_unknown_fields)] 追加"]} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage4-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage4-review-context.json new file mode 100644 index 0000000..baf8f87 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage4-review-context.json @@ -0,0 +1,23 @@ +{ + "stage": 4, + "type": "セキュリティレビュー", + "must_fix": [ + {"id": "M1", "title": "シンボリックリンク/ハードリンクによるパストラバーサルが未対策"}, + {"id": "M2", "title": "canonicalize ベースの検証と擬似コードが矛盾 — 一本化が必要"}, + {"id": "M3", "title": "圧縮爆弾対策が未設計 — 展開サイズ/エントリ数の上限制御が必要"} + ], + "should_fix": [ + {"id": "S1", "title": "state.json/export_meta.json 内の絶対パス漏洩リスク"}, + {"id": "S2", "title": "ハードリンクエントリの処理方針未定義"}, + {"id": "S3", "title": "export_meta.json デシリアライズ後のバリデーション不足"}, + {"id": "S4", "title": "symbols.db のパス情報漏洩リスク注意書き"}, + {"id": "S5", "title": "commandindex.toml のエクスポート対象外明記"} + ], + "nice_to_have": [ + {"id": "N1", "title": "展開先ファイルのパーミッション固定化"}, + {"id": "N2", "title": ".gitignore に *.local.toml パターン追加"}, + {"id": "N3", "title": "アーカイブのチェックサム検証"}, + {"id": "N4", "title": "Unicode 正規化によるパス偽装対策"} + ], + "must_fix_count": 3 +} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage5-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage5-review-context.json new file mode 100644 index 0000000..ef79fc2 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage5-review-context.json @@ -0,0 +1 @@ +{"stage": 5, "status": "skipped", "reason": "Codex サーバーエラーのためスキップ。Stage 1-4 で Must Fix 11件を全て反映済み。"} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage6-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage6-apply-result.json new file mode 100644 index 0000000..2b453b3 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage6-apply-result.json @@ -0,0 +1 @@ +{"stage": 6, "status": "skipped", "reason": "Stage 5 スキップのため"} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage7-review-context.json b/dev-reports/issue/77/multi-stage-design-review/stage7-review-context.json new file mode 100644 index 0000000..eaa74c4 --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage7-review-context.json @@ -0,0 +1 @@ +{"stage": 7, "status": "skipped", "reason": "Codex サーバーエラーのためスキップ。"} diff --git a/dev-reports/issue/77/multi-stage-design-review/stage8-apply-result.json b/dev-reports/issue/77/multi-stage-design-review/stage8-apply-result.json new file mode 100644 index 0000000..b42214e --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/stage8-apply-result.json @@ -0,0 +1 @@ +{"stage": 8, "status": "skipped", "reason": "Stage 7 スキップのため"} diff --git a/dev-reports/issue/77/multi-stage-design-review/summary-report.md b/dev-reports/issue/77/multi-stage-design-review/summary-report.md new file mode 100644 index 0000000..67326ba --- /dev/null +++ b/dev-reports/issue/77/multi-stage-design-review/summary-report.md @@ -0,0 +1,43 @@ +# Issue #77 マルチステージ設計レビュー サマリーレポート + +## 実施ステージ + +| Stage | 種別 | エージェント | Must Fix | Should Fix | Nice to Have | +|-------|------|-------------|----------|-----------|-------------| +| 1 | 設計原則 (SOLID/KISS/YAGNI/DRY) | Claude opus | 2 | 3 | 5 | +| 2 | 整合性 | Claude opus | 2 | 4 | 3 | +| 3 | 影響分析 | Claude opus | 4 | 5 | 4 | +| 4 | セキュリティ | Claude opus | 3 | 5 | 4 | +| 5-8 | 2回目 | Codex | スキップ（サーバーエラー） | - | - | + +## Must Fix 対応サマリー (11件) + +### 設計原則 (Stage 1) +1. **YAGNI**: StatusOptions 構造体 → verify: bool 引数に簡素化 +2. **SRP**: snapshot.rs の責務を ExportMeta 定義+読み書きに限定 + +### 整合性 (Stage 2) +3. **定数統一**: CLI層は commandindex_dir() ヘルパー、Indexer層は既存定数 +4. **パターン準拠**: status::run() は format 別引数のまま verify: bool 追加（CleanOptions パターン） + +### 影響分析 (Stage 3) +5. **破壊的変更最小化**: テスト修正は verify: false 追加の4箇所のみ +6. **import後整合性**: update コマンドとの整合性を統合テストで検証 +7. **相対パス検証**: export 前に tantivy ドキュメント内パスが相対パスであることを確認 +8. **相対パス検証**: export 前に symbols.db 内パスが相対パスであることを確認 + +### セキュリティ (Stage 4) +9. **シンボリックリンク**: validate_entry_type() でSymlink/Link エントリを即座に拒否 +10. **パス検証統一**: canonicalize() は使わず、文字列レベル+components()検証に一本化 +11. **圧縮爆弾対策**: 展開サイズ上限(1GB), エントリ数上限(10000) を追加 + +## 設計方針書の主要改善点 + +- ExportMeta から index_root を削除（情報漏洩防止） +- state.json のパック時サニタイズ +- ExportResult/ImportResult 構造体導入（既存パターン準拠） +- エラー型に std::error::Error + From 実装を明記 +- doc comment を英語に統一 +- テストファイル命名を既存パターンに統一 +- git hash 取得関数の分離（DIP, テスタビリティ） +- #[serde(deny_unknown_fields)] 追加 diff --git a/dev-reports/issue/77/pm-auto-dev/iteration-1/acceptance-result.json b/dev-reports/issue/77/pm-auto-dev/iteration-1/acceptance-result.json new file mode 100644 index 0000000..595e032 --- /dev/null +++ b/dev-reports/issue/77/pm-auto-dev/iteration-1/acceptance-result.json @@ -0,0 +1,7 @@ +{ + "status": "all_passed", + "passed_count": 14, + "failed_count": 0, + "added_tests": ["import_git_commit_hash_mismatch_shows_warning"], + "summary": "全14件の受け入れ基準をパス。AC-7のテストを1件追加。" +} diff --git a/dev-reports/issue/77/pm-auto-dev/iteration-1/codex-review-result.json b/dev-reports/issue/77/pm-auto-dev/iteration-1/codex-review-result.json new file mode 100644 index 0000000..36f5e8c --- /dev/null +++ b/dev-reports/issue/77/pm-auto-dev/iteration-1/codex-review-result.json @@ -0,0 +1 @@ +{"critical":[{"file":"src/cli/import_index.rs","line":179,"title":"`--force` import deletes the existing index before the archive is validated","detail":"`run()` removes `.commandindex/` immediately when `--force` is set, then starts parsing and extracting the archive. Any later failure such as malformed gzip/tar, incompatible `export_meta.json`, invalid `state.json`, or an I/O error leaves the user with the old index already destroyed. This is both a data-loss bug and a denial-of-service vector for untrusted archives.","recommendation":"Extract into a temporary directory, validate the archive contents completely, rewrite `state.json`, and only then replace the destination atomically. Keep the original `.commandindex/` untouched until validation succeeds."},{"file":"src/cli/import_index.rs","line":109,"title":"Windows path prefixes are not rejected during archive path validation","detail":"`validate_entry_path()` rejects absolute paths and `..`, but it does not reject `Component::Prefix` entries such as `C:foo` or UNC-prefixed paths on Windows. `target_dir.join(entry_path)` can therefore escape the intended destination on Windows even though `is_absolute()` is false, turning import into an arbitrary file write primitive on that platform.","recommendation":"Reject `Component::Prefix` explicitly in `validate_entry_path()`, and consider allowing only a strict set of relative normal components before joining with the destination path."}],"warnings":[{"file":"src/cli/import_index.rs","line":187,"title":"Failed imports can leave a partial `.commandindex/` behind","detail":"Cleanup is only performed for some error branches (`MAX_ENTRY_COUNT`, decompression limit, missing/incompatible `export_meta.json`). Errors from path traversal, symlink detection, `read_to_end`, `write`, `set_permissions`, or `serde_json::from_str(state.json)` return early without removing the partially extracted directory. Subsequent commands may observe a corrupted index layout.","recommendation":"Route all post-creation failures through a single cleanup path, or better, extract into a temporary directory and rename only on success."},{"file":"src/cli/status.rs","line":285,"title":"`status --verify` cannot report corrupted `state.json` because `run()` fails before verification runs","detail":"`run()` always loads `state.json` and checks its schema before it considers the `verify` flag. If `state.json` is missing or incompatible, `status --verify` exits with an error instead of returning the structured verification result produced by `run_verify()`. That weakens diagnostics exactly for the failure mode `--verify` is supposed to help investigate.","recommendation":"When `verify` is requested, execute the verification path first and report its findings even if `state.json` cannot be loaded for the normal status summary."}],"summary":"Review focused on `src/cli/export.rs`, `src/cli/import_index.rs`, `src/indexer/snapshot.rs`, `src/cli/status.rs`, and `src/main.rs`. Two fix-required issues were found in import: destructive `--force` behavior before validation and incomplete path validation on Windows. Additional warnings were found around partial extraction cleanup and `status --verify` diagnostics. No `unwrap()`/`expect()` panics or `unsafe` blocks were found in the reviewed production files.","requires_fix":true} diff --git a/dev-reports/issue/77/pm-auto-dev/iteration-1/refactor-result.json b/dev-reports/issue/77/pm-auto-dev/iteration-1/refactor-result.json new file mode 100644 index 0000000..3828f89 --- /dev/null +++ b/dev-reports/issue/77/pm-auto-dev/iteration-1/refactor-result.json @@ -0,0 +1,7 @@ +{ + "status": "success", + "changes": [ + "current_git_hash 関数を export.rs/import_index.rs から snapshot.rs に統合（DRY）", + "status.rs のハードコードパスを commandindex_dir() に統一（一貫性）" + ] +} diff --git a/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-context.json b/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-context.json new file mode 100644 index 0000000..80584e2 --- /dev/null +++ b/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-context.json @@ -0,0 +1,18 @@ +{ + "issue_number": 77, + "title": "インデックス共有モード（export/import/verify）", + "design_policy": "dev-reports/design/issue-77-design-policy.md", + "work_plan": "dev-reports/issue/77/work-plan.md", + "implementation_order": [ + "Task 1.1: ExportMeta構造体 (src/indexer/snapshot.rs)", + "Task 1.2: エラー型・結果型定義", + "Task 1.3: Cargo.toml依存追加 (tar, flate2)", + "Task 3.1: パストラバーサル検証ロジック", + "Task 2.1: エクスポートロジック (src/cli/export.rs)", + "Task 3.2: インポートロジック (src/cli/import_index.rs)", + "Task 4.1: status --verify 拡張", + "Task 5.1: main.rs + cli/mod.rs 統合", + "Task 5.2: 既存テスト修正", + "Task 6.1: E2Eテスト" + ] +} diff --git a/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-result.json b/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-result.json new file mode 100644 index 0000000..8581563 --- /dev/null +++ b/dev-reports/issue/77/pm-auto-dev/iteration-1/tdd-result.json @@ -0,0 +1,28 @@ +{ + "status": "success", + "new_files": [ + "src/indexer/snapshot.rs", + "src/cli/export.rs", + "src/cli/import_index.rs", + "tests/cli_export.rs", + "tests/cli_import.rs", + "tests/e2e_export_import.rs", + "tests/e2e_verify.rs" + ], + "modified_files": [ + "Cargo.toml", + "src/indexer/mod.rs", + "src/cli/mod.rs", + "src/cli/status.rs", + "src/main.rs", + "tests/cli_args.rs", + "tests/cli_status.rs" + ], + "test_count": 20, + "quality_checks": { + "build": "pass", + "clippy": "pass (0 warnings)", + "test": "pass (0 failures)", + "fmt": "pass (no diff)" + } +} diff --git a/dev-reports/issue/77/work-plan.md b/dev-reports/issue/77/work-plan.md new file mode 100644 index 0000000..00054fb --- /dev/null +++ b/dev-reports/issue/77/work-plan.md @@ -0,0 +1,207 @@ +# 作業計画書 - Issue #77: インデックス共有モード + +## Issue概要 + +**Issue番号**: #77 +**タイトル**: [Feature] インデックス共有モード（--shared / CI連携） +**サイズ**: L（新規サブコマンド2つ + status拡張 + セキュリティ実装） +**優先度**: Medium +**依存Issue**: #76 チーム共有設定ファイル（完了済み） + +## タスク分解 + +### Phase 1: 基盤（データ構造・型定義） + +#### Task 1.1: ExportMeta 構造体とスナップショットモジュール +- **成果物**: `src/indexer/snapshot.rs` +- **依存**: なし +- **内容**: + - `ExportMeta` 構造体（export_format_version, commandindex_version, git_commit_hash, exported_at） + - `#[serde(deny_unknown_fields)]` 付与 + - `EXPORT_FORMAT_VERSION` 定数（初期値: 1） + - `EXPORT_META_FILE` 定数（"export_meta.json"） + - `ExportMeta::save()` / `ExportMeta::load()` メソッド + - `src/indexer/mod.rs` に `pub mod snapshot;` 追加 +- **テスト**: ExportMeta のシリアライズ/デシリアライズ、deny_unknown_fields の検証 + +#### Task 1.2: エラー型定義 +- **成果物**: `src/cli/export.rs`（エラー型部分）, `src/cli/import_index.rs`（エラー型部分） +- **依存**: Task 1.1 +- **内容**: + - `ExportError` enum + `fmt::Display` + `std::error::Error` + `From` 実装 + - `ImportError` enum（SymlinkDetected, DecompressionBomb 含む）+ 同様の実装 + - `ExportResult` / `ImportResult` 構造体 + - `ExportOptions` / `ImportOptions` 構造体 +- **テスト**: エラー型の Display 出力確認 + +#### Task 1.3: Cargo.toml 依存追加 +- **成果物**: `Cargo.toml` +- **依存**: なし +- **内容**: + - `tar = "0.4"` 追加 + - `flate2 = "1"` 追加（default features = miniz_oxide） +- **テスト**: `cargo build` 成功確認 + +### Phase 2: コア実装（エクスポート） + +#### Task 2.1: エクスポートロジック実装 +- **成果物**: `src/cli/export.rs` +- **依存**: Task 1.1, 1.2, 1.3 +- **内容**: + - `pub fn run(path: &Path, output: &Path, options: &ExportOptions) -> Result` + - `fn current_git_hash(repo_path: &Path) -> Option` ユーティリティ関数 + - .commandindex/ 存在確認 + - IndexState::load() でインデックス状態読み込み + - state.json の index_root サニタイズ（placeholder 置換してパック） + - tar::Builder + flate2::GzEncoder ストリーミング圧縮 + - config.local.toml 除外ロジック + - embeddings.db の --with-embeddings 制御 + - 出力ファイルサイズ計算 +- **テスト**: TDDで実装（テスト先行） + +#### Task 2.2: エクスポートテスト +- **成果物**: `tests/cli_export.rs` +- **依存**: Task 2.1 +- **内容**: + - export 基本動作テスト（インデックス作成 → export → アーカイブ内容検証） + - NotInitialized エラーテスト + - config.local.toml 除外検証 + - embeddings.db デフォルト除外検証 + - --with-embeddings 時の embeddings.db 含む検証 + +### Phase 3: コア実装（インポート） + +#### Task 3.1: パストラバーサル検証ロジック +- **成果物**: `src/cli/import_index.rs`（検証関数部分） +- **依存**: Task 1.2 +- **内容**: + - `fn validate_entry_path(entry_path: &Path, target_dir: &Path) -> Result` + - `fn validate_entry_type(entry: &tar::Entry) -> Result<(), ImportError>` + - 絶対パス拒否、`..` コンポーネント拒否 + - Symlink/Link エントリ拒否 + - 累積サイズ/エントリ数チェックロジック +- **テスト**: TDDで実装（パストラバーサル、シンボリックリンク、圧縮爆弾の各テストケース） + +#### Task 3.2: インポートロジック実装 +- **成果物**: `src/cli/import_index.rs` +- **依存**: Task 1.1, 1.2, 1.3, 3.1 +- **内容**: + - `pub fn run(path: &Path, archive: &Path, options: &ImportOptions) -> Result` + - アーカイブファイル存在確認 + - 既存 .commandindex/ チェック + --force 制御 + - ストリーミング展開（各エントリにセキュリティチェック適用） + - export_meta.json 読み込み + バージョン互換性チェック + - state.json の index_root 書き換え + - git hash 比較 + 不一致警告 + - tantivy インデックスオープン確認 + - パーミッション固定（0o644/0o755） +- **テスト**: TDDで実装 + +#### Task 3.3: インポートテスト +- **成果物**: `tests/cli_import.rs` +- **依存**: Task 3.2 +- **内容**: + - import 基本動作テスト + - 既存インデックスありで --force なしのエラー + - --force での上書きインポート + - パストラバーサル検出テスト（`../` パス） + - シンボリックリンクエントリ拒否テスト + - ハードリンクエントリ拒否テスト + - 圧縮爆弾検出テスト + - export_format_version 不一致エラー + - コミットハッシュ不一致警告 + +### Phase 4: Status --verify 拡張 + +#### Task 4.1: verify ロジック実装 +- **成果物**: `src/cli/status.rs`（変更） +- **依存**: なし（他 Phase と並行可能） +- **内容**: + - `run()` シグネチャに `verify: bool` 引数追加 + - `VerifyResult` / `VerifyIssue` / `VerifySeverity` 構造体追加 + - verify ロジック: state確認 → tantivy確認 → manifest確認 → symbols確認 + - Human/Json 両フォーマットでの verify 結果出力 +- **テスト**: TDDで実装 + +#### Task 4.2: verify テスト +- **成果物**: `tests/e2e_verify.rs` +- **依存**: Task 4.1 +- **内容**: + - 正常インデックスの verify パス + - 破損インデックスの verify エラー検出 + +### Phase 5: CLI統合 + +#### Task 5.1: main.rs + cli/mod.rs 統合 +- **成果物**: `src/main.rs`, `src/cli/mod.rs` +- **依存**: Task 2.1, 3.2, 4.1 +- **内容**: + - Commands enum に `Export` / `Import` バリアント追加（英語 doc comment） + - Status バリアントに `verify: bool` 追加 + - main.rs の match 分岐に Export / Import 処理追加 + - Status 分岐の run() 呼び出しに verify 引数追加 + - cli/mod.rs に `pub mod export;` `pub mod import_index;` 追加 + +#### Task 5.2: 既存テスト修正 +- **成果物**: `tests/cli_args.rs`, `tests/cli_status.rs` +- **依存**: Task 5.1 +- **内容**: + - cli_args.rs: help_flag_shows_usage に export/import 検証追加 + - cli_status.rs: run() 呼び出し3箇所に verify: false 追加 + +### Phase 6: E2Eテスト + +#### Task 6.1: export → import → search E2Eテスト +- **成果物**: `tests/e2e_export_import.rs` +- **依存**: Task 5.1 +- **内容**: + - インデックス作成 → export → import → search の完全フロー + - import 後に tantivy インデックスがオープンできることの確認 + - import 後に update が正常動作することの確認 + +### Phase 7: 品質チェック + +#### Task 7.1: 最終品質チェック +- **依存**: 全タスク完了 +- **内容**: + - `cargo build` エラー0件 + - `cargo clippy --all-targets -- -D warnings` 警告0件 + - `cargo test --all` 全テストパス + - `cargo fmt --all -- --check` 差分なし + +## タスク依存関係図 + +``` +Task 1.1 (ExportMeta) ──┐ +Task 1.2 (エラー型) ──┤ +Task 1.3 (Cargo.toml) ──┤ + ├──→ Task 2.1 (export) ──→ Task 2.2 (export テスト) + ├──→ Task 3.1 (パス検証) ──→ Task 3.2 (import) ──→ Task 3.3 (import テスト) + │ +Task 4.1 (verify) ───────┤──→ Task 4.2 (verify テスト) + │ + └──→ Task 5.1 (main.rs 統合) ──→ Task 5.2 (既存テスト修正) + ──→ Task 6.1 (E2E テスト) + ──→ Task 7.1 (品質チェック) +``` + +## TDD実装順序 + +1. **Task 1.1** → テスト: ExportMeta シリアライズ +2. **Task 1.2 + 1.3** → テスト: エラー型, ビルド確認 +3. **Task 3.1** → テスト: パストラバーサル検証（セキュリティテスト先行） +4. **Task 2.1 + 2.2** → テスト: export 基本動作 +5. **Task 3.2 + 3.3** → テスト: import 基本動作 + セキュリティ +6. **Task 4.1 + 4.2** → テスト: verify +7. **Task 5.1 + 5.2** → テスト: CLI統合 + 既存テスト修正 +8. **Task 6.1** → テスト: E2E +9. **Task 7.1** → 品質チェック + +## Definition of Done + +- [ ] すべてのタスクが完了 +- [ ] `cargo test --all` 全テストパス +- [ ] `cargo clippy --all-targets -- -D warnings` 警告0件 +- [ ] `cargo fmt --all -- --check` 差分なし +- [ ] セキュリティテスト（パストラバーサル、シンボリックリンク、圧縮爆弾）全パス +- [ ] E2E テスト（export → import → search）パス diff --git a/src/cli/export.rs b/src/cli/export.rs new file mode 100644 index 0000000..b077f04 --- /dev/null +++ b/src/cli/export.rs @@ -0,0 +1,196 @@ +use std::fmt; +use std::fs::File; +use std::io::Write; +use std::path::{Path, PathBuf}; + +use chrono::Utc; +use flate2::Compression; +use flate2::write::GzEncoder; +use tar::Builder; + +use crate::indexer::manifest::ManifestError; +use crate::indexer::snapshot::{ + EXPORT_FORMAT_VERSION, EXPORT_META_FILE, ExportMeta, current_git_hash, +}; +use crate::indexer::state::{IndexState, StateError}; + +/// Export options +pub struct ExportOptions { + pub with_embeddings: bool, +} + +/// Export result +#[derive(Debug)] +pub struct ExportResult { + pub output_path: PathBuf, + pub archive_size: u64, + pub git_commit_hash: Option, +} + +/// Export error +#[derive(Debug)] +pub enum ExportError { + NotInitialized, + Io(std::io::Error), + State(StateError), + Manifest(ManifestError), + Serialize(serde_json::Error), + GitError(String), +} + +impl fmt::Display for ExportError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ExportError::NotInitialized => { + write!(f, "Index not initialized. Run `commandindex index` first.") + } + ExportError::Io(e) => write!(f, "IO error: {e}"), + ExportError::State(e) => write!(f, "State error: {e}"), + ExportError::Manifest(e) => write!(f, "Manifest error: {e}"), + ExportError::Serialize(e) => write!(f, "Serialization error: {e}"), + ExportError::GitError(msg) => write!(f, "Git error: {msg}"), + } + } +} + +impl std::error::Error for ExportError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ExportError::Io(e) => Some(e), + ExportError::State(e) => Some(e), + ExportError::Serialize(e) => Some(e), + _ => None, + } + } +} + +impl From for ExportError { + fn from(e: std::io::Error) -> Self { + ExportError::Io(e) + } +} + +impl From for ExportError { + fn from(e: StateError) -> Self { + ExportError::State(e) + } +} + +impl From for ExportError { + fn from(e: ManifestError) -> Self { + ExportError::Manifest(e) + } +} + +impl From for ExportError { + fn from(e: serde_json::Error) -> Self { + ExportError::Serialize(e) + } +} + +/// Placeholder for sanitized index_root in exported state.json +const INDEX_ROOT_PLACEHOLDER: &str = "__COMMANDINDEX_EXPORT_PLACEHOLDER__"; + +/// Export index as portable tar.gz archive +pub fn run( + path: &Path, + output: &Path, + options: &ExportOptions, +) -> Result { + let ci_dir = crate::indexer::commandindex_dir(path); + + // 1. Check .commandindex/ exists + if !IndexState::exists(&ci_dir) { + return Err(ExportError::NotInitialized); + } + + // 2. Load index state + let state = IndexState::load(&ci_dir)?; + state.check_schema_version()?; + + // 3. Get git commit hash + let git_hash = current_git_hash(path); + + // 4. Build ExportMeta + let meta = ExportMeta { + export_format_version: EXPORT_FORMAT_VERSION, + commandindex_version: env!("CARGO_PKG_VERSION").to_string(), + git_commit_hash: git_hash.clone(), + exported_at: Utc::now(), + }; + + // 5. Create tar.gz archive + let file = File::create(output)?; + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + // 5a. Add export_meta.json first + let meta_json = serde_json::to_string_pretty(&meta)?; + add_bytes_to_tar(&mut builder, EXPORT_META_FILE, meta_json.as_bytes())?; + + // 5b. Add state.json with sanitized index_root + let mut sanitized_state = state.clone(); + sanitized_state.index_root = PathBuf::from(INDEX_ROOT_PLACEHOLDER); + let state_json = serde_json::to_string_pretty(&sanitized_state)?; + add_bytes_to_tar(&mut builder, "state.json", state_json.as_bytes())?; + + // 5c. Add manifest.json if it exists + let manifest_path = ci_dir.join("manifest.json"); + if manifest_path.exists() { + builder.append_path_with_name(&manifest_path, "manifest.json")?; + } + + // 5d. Add symbols.db if it exists + let symbols_path = ci_dir.join("symbols.db"); + if symbols_path.exists() { + builder.append_path_with_name(&symbols_path, "symbols.db")?; + } + + // 5e. Add tantivy/ directory recursively + let tantivy_dir = ci_dir.join("tantivy"); + if tantivy_dir.is_dir() { + builder.append_dir_all("tantivy", &tantivy_dir)?; + } + + // 5f. Add embeddings.db if --with-embeddings + if options.with_embeddings { + let embeddings_path = ci_dir.join("embeddings.db"); + if embeddings_path.exists() { + builder.append_path_with_name(&embeddings_path, "embeddings.db")?; + } + } + + // Note: config.local.toml is always excluded (not added) + + // 6. Finalize archive + let encoder = builder.into_inner()?; + encoder.finish()?; + + // 7. Get archive size + let archive_size = std::fs::metadata(output)?.len(); + + Ok(ExportResult { + output_path: output.to_path_buf(), + archive_size, + git_commit_hash: git_hash, + }) +} + +/// Helper to add bytes as a file entry to a tar archive +fn add_bytes_to_tar( + builder: &mut Builder, + name: &str, + data: &[u8], +) -> Result<(), std::io::Error> { + let mut header = tar::Header::new_gnu(); + header.set_size(data.len() as u64); + header.set_mode(0o644); + header.set_mtime( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(), + ); + header.set_cksum(); + builder.append_data(&mut header, name, data) +} diff --git a/src/cli/import_index.rs b/src/cli/import_index.rs new file mode 100644 index 0000000..54e4fd0 --- /dev/null +++ b/src/cli/import_index.rs @@ -0,0 +1,375 @@ +use std::fmt; +use std::fs::File; +use std::io::Read; +use std::path::{Component, Path, PathBuf}; + +use flate2::read::GzDecoder; +use tar::Archive; + +use crate::indexer::snapshot::{ + EXPORT_FORMAT_VERSION, EXPORT_META_FILE, ExportMeta, current_git_hash, +}; +use crate::indexer::state::{IndexState, StateError}; + +/// Maximum cumulative decompressed size (1 GB) +const MAX_DECOMPRESS_SIZE: u64 = 1_073_741_824; + +/// Maximum number of entries in the archive +const MAX_ENTRY_COUNT: u64 = 10_000; + +/// Import options +pub struct ImportOptions { + pub force: bool, +} + +/// Import result +#[derive(Debug)] +pub struct ImportResult { + pub imported_files: u64, + pub git_hash_match: bool, + pub warnings: Vec, +} + +/// Import error +#[derive(Debug)] +pub enum ImportError { + Io(std::io::Error), + ExistingIndex(PathBuf), + PathTraversal(String), + SymlinkDetected(PathBuf), + InvalidArchive(String), + IncompatibleVersion { expected: u32, found: u32 }, + DecompressionBomb { limit: u64 }, + State(StateError), + Deserialize(serde_json::Error), +} + +impl fmt::Display for ImportError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ImportError::Io(e) => write!(f, "IO error: {e}"), + ImportError::ExistingIndex(p) => { + write!( + f, + "Index already exists at {}. Use --force to overwrite.", + p.display() + ) + } + ImportError::PathTraversal(msg) => write!(f, "Path traversal detected: {msg}"), + ImportError::SymlinkDetected(p) => { + write!(f, "Symlink/hardlink entry detected: {}", p.display()) + } + ImportError::InvalidArchive(msg) => write!(f, "Invalid archive: {msg}"), + ImportError::IncompatibleVersion { expected, found } => { + write!( + f, + "Incompatible export format version: expected <= {expected}, found {found}" + ) + } + ImportError::DecompressionBomb { limit } => { + write!(f, "Decompression bomb: exceeded {limit} bytes limit") + } + ImportError::State(e) => write!(f, "State error: {e}"), + ImportError::Deserialize(e) => write!(f, "Deserialization error: {e}"), + } + } +} + +impl std::error::Error for ImportError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ImportError::Io(e) => Some(e), + ImportError::State(e) => Some(e), + ImportError::Deserialize(e) => Some(e), + _ => None, + } + } +} + +impl From for ImportError { + fn from(e: std::io::Error) -> Self { + ImportError::Io(e) + } +} + +impl From for ImportError { + fn from(e: StateError) -> Self { + ImportError::State(e) + } +} + +impl From for ImportError { + fn from(e: serde_json::Error) -> Self { + ImportError::Deserialize(e) + } +} + +/// Validate an entry path for path traversal attacks +fn validate_entry_path(entry_path: &Path, target_dir: &Path) -> Result { + // 1. Reject absolute paths + if entry_path.is_absolute() { + return Err(ImportError::PathTraversal(format!( + "absolute path: {}", + entry_path.display() + ))); + } + + // 2. Reject ".." and prefix components + for component in entry_path.components() { + match component { + Component::ParentDir => { + return Err(ImportError::PathTraversal(format!( + "parent dir: {}", + entry_path.display() + ))); + } + Component::Prefix(_) => { + return Err(ImportError::PathTraversal(format!( + "path prefix: {}", + entry_path.display() + ))); + } + _ => {} + } + } + + // 3. Build full path + let full_path = target_dir.join(entry_path); + + Ok(full_path) +} + +/// Validate entry type - reject symlinks and hardlinks +fn validate_entry_type(entry_type: tar::EntryType, entry_path: &Path) -> Result<(), ImportError> { + match entry_type { + tar::EntryType::Symlink | tar::EntryType::Link => { + Err(ImportError::SymlinkDetected(entry_path.to_path_buf())) + } + _ => Ok(()), + } +} + +/// Import index from tar.gz archive +pub fn run( + path: &Path, + archive: &Path, + options: &ImportOptions, +) -> Result { + let ci_dir = crate::indexer::commandindex_dir(path); + + // 1. Check archive exists + if !archive.exists() { + return Err(ImportError::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("Archive not found: {}", archive.display()), + ))); + } + + // 2. Check existing .commandindex/ + if ci_dir.exists() { + if !options.force { + return Err(ImportError::ExistingIndex(ci_dir.clone())); + } + // --force: verify not a symlink before removing + let metadata = std::fs::symlink_metadata(&ci_dir)?; + if metadata.file_type().is_symlink() { + return Err(ImportError::SymlinkDetected(ci_dir.clone())); + } + } + + // 3. Extract to a temporary directory first, then swap on success + let temp_dir = ci_dir.with_file_name(".commandindex_import_tmp"); + if temp_dir.exists() { + std::fs::remove_dir_all(&temp_dir)?; + } + std::fs::create_dir_all(&temp_dir)?; + + // 4. Extract archive into temp_dir with security checks + let result = extract_and_validate(path, archive, &temp_dir); + + match result { + Ok(import_result) => { + // All validation passed — atomically swap directories + if ci_dir.exists() { + std::fs::remove_dir_all(&ci_dir)?; + } + std::fs::rename(&temp_dir, &ci_dir)?; + Ok(import_result) + } + Err(e) => { + // Cleanup temp dir on any error + let _ = std::fs::remove_dir_all(&temp_dir); + Err(e) + } + } +} + +/// Extract archive contents into target directory and validate +fn extract_and_validate( + base_path: &Path, + archive: &Path, + target_dir: &Path, +) -> Result { + let file = File::open(archive)?; + let decoder = GzDecoder::new(file); + let mut tar_archive = Archive::new(decoder); + + let mut total_size: u64 = 0; + let mut entry_count: u64 = 0; + let mut imported_files: u64 = 0; + let mut warnings = Vec::new(); + + let entries = tar_archive + .entries() + .map_err(|e| ImportError::InvalidArchive(format!("Failed to read entries: {e}")))?; + + for entry_result in entries { + let mut entry = entry_result + .map_err(|e| ImportError::InvalidArchive(format!("Failed to read entry: {e}")))?; + + // Entry count check + entry_count += 1; + if entry_count > MAX_ENTRY_COUNT { + return Err(ImportError::DecompressionBomb { + limit: MAX_ENTRY_COUNT, + }); + } + + let entry_path = entry + .path() + .map_err(|e| ImportError::InvalidArchive(format!("Invalid path: {e}")))? + .to_path_buf(); + + let entry_type = entry.header().entry_type(); + + // Skip directory entries - we'll create dirs as needed + if entry_type == tar::EntryType::Directory { + continue; + } + + // Security: validate entry type (reject symlinks/hardlinks) + validate_entry_type(entry_type, &entry_path)?; + + // Security: validate path (reject traversal) + let full_path = validate_entry_path(&entry_path, target_dir)?; + + // Header size pre-check (early rejection, may be forged) + let header_size = entry.header().size().unwrap_or(0); + total_size = total_size.saturating_add(header_size); + if total_size > MAX_DECOMPRESS_SIZE { + return Err(ImportError::DecompressionBomb { + limit: MAX_DECOMPRESS_SIZE, + }); + } + + // Create parent directories + if let Some(parent) = full_path.parent() { + std::fs::create_dir_all(parent)?; + } + + // Extract file with fixed permissions + let mut content = Vec::new(); + entry.read_to_end(&mut content)?; + + // Verify actual data size (header size may be forged) + let actual_size = content.len() as u64; + if actual_size > header_size { + // Re-adjust total_size with actual data + total_size = total_size + .saturating_sub(header_size) + .saturating_add(actual_size); + if total_size > MAX_DECOMPRESS_SIZE { + return Err(ImportError::DecompressionBomb { + limit: MAX_DECOMPRESS_SIZE, + }); + } + } + + std::fs::write(&full_path, &content)?; + + // Fix permissions (Unix only) + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = if full_path + .extension() + .is_some_and(|e| e == "sh" || e == "exe") + { + 0o755 + } else { + 0o644 + }; + std::fs::set_permissions(&full_path, std::fs::Permissions::from_mode(mode))?; + } + + imported_files += 1; + } + + // 5. Load and validate export_meta.json + let meta_path = target_dir.join(EXPORT_META_FILE); + if !meta_path.exists() { + return Err(ImportError::InvalidArchive( + "Missing export_meta.json".to_string(), + )); + } + + let meta = ExportMeta::load(&meta_path).map_err(|e| { + ImportError::InvalidArchive(format!("Failed to load export_meta.json: {e}")) + })?; + + // Version compatibility check (forward-compatible policy) + if meta.export_format_version > EXPORT_FORMAT_VERSION { + return Err(ImportError::IncompatibleVersion { + expected: EXPORT_FORMAT_VERSION, + found: meta.export_format_version, + }); + } + + // Validate commandindex_version length + if meta.commandindex_version.len() > 64 { + return Err(ImportError::InvalidArchive( + "commandindex_version too long".to_string(), + )); + } + + // 6. Rewrite state.json index_root to import target path + let state_path = target_dir.join("state.json"); + if state_path.exists() { + let state_content = std::fs::read_to_string(&state_path)?; + let mut state: IndexState = serde_json::from_str(&state_content)?; + + // Always set index_root to import target path + state.index_root = base_path.to_path_buf(); + state.save(target_dir)?; + } + + // 7. Check git hash match + let current_hash = current_git_hash(base_path); + let git_hash_match = match (&meta.git_commit_hash, ¤t_hash) { + (Some(export_hash), Some(current)) => { + if export_hash != current { + warnings.push(format!( + "Git commit hash mismatch: exported={}, current={}", + export_hash, current + )); + false + } else { + true + } + } + (Some(_), None) => { + warnings.push("Could not determine current git commit hash".to_string()); + false + } + (None, _) => { + // No hash recorded in export + true + } + }; + + Ok(ImportResult { + imported_files, + git_hash_match, + warnings, + }) +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index fd9721e..389822e 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2,6 +2,8 @@ pub mod clean; pub mod config; pub mod context; pub mod embed; +pub mod export; +pub mod import_index; pub mod index; pub mod search; pub mod status; diff --git a/src/cli/status.rs b/src/cli/status.rs index 4a283e5..178039f 100644 --- a/src/cli/status.rs +++ b/src/cli/status.rs @@ -73,6 +73,38 @@ pub struct StatusInfo { pub symbol_count: u64, } +/// Verify severity level +#[derive(Debug, Serialize)] +pub enum VerifySeverity { + Error, + Warning, +} + +/// A single verify issue +#[derive(Debug, Serialize)] +pub struct VerifyIssue { + pub component: String, + pub severity: VerifySeverity, + pub message: String, +} + +/// Integrity verification result +#[derive(Debug, Serialize)] +pub struct VerifyResult { + pub state_valid: bool, + pub tantivy_valid: bool, + pub manifest_valid: bool, + pub symbols_valid: bool, + pub issues: Vec, +} + +impl VerifyResult { + /// Overall pass/fail + pub fn is_ok(&self) -> bool { + self.state_valid && self.tantivy_valid && self.manifest_valid && self.symbols_valid + } +} + /// ディレクトリサイズを再帰的に計算する /// /// エラーが発生したエントリはスキップし、取得可能なファイルサイズの合計を返す。 @@ -140,13 +172,115 @@ fn get_symbol_count(base_path: &Path) -> u64 { } } +/// Run integrity verification on the index +fn run_verify(path: &Path) -> VerifyResult { + let ci_dir = crate::indexer::commandindex_dir(path); + let mut issues = Vec::new(); + + // 1. state.json check + let state_valid = match IndexState::load(&ci_dir) { + Ok(state) => match state.check_schema_version() { + Ok(()) => true, + Err(e) => { + issues.push(VerifyIssue { + component: "state".to_string(), + severity: VerifySeverity::Error, + message: format!("Schema version check failed: {e}"), + }); + false + } + }, + Err(e) => { + issues.push(VerifyIssue { + component: "state".to_string(), + severity: VerifySeverity::Error, + message: format!("Failed to load state.json: {e}"), + }); + false + } + }; + + // 2. tantivy/ directory check + let tantivy_dir = ci_dir.join("tantivy"); + let tantivy_valid = if tantivy_dir.is_dir() { + match tantivy::Index::open_in_dir(&tantivy_dir) { + Ok(_) => true, + Err(e) => { + issues.push(VerifyIssue { + component: "tantivy".to_string(), + severity: VerifySeverity::Error, + message: format!("Failed to open tantivy index: {e}"), + }); + false + } + } + } else { + issues.push(VerifyIssue { + component: "tantivy".to_string(), + severity: VerifySeverity::Error, + message: "tantivy/ directory not found".to_string(), + }); + false + }; + + // 3. manifest.json check + let manifest_valid = match Manifest::load(&ci_dir) { + Ok(_) => true, + Err(e) => { + issues.push(VerifyIssue { + component: "manifest".to_string(), + severity: VerifySeverity::Warning, + message: format!("Failed to load manifest.json: {e}"), + }); + false + } + }; + + // 4. symbols.db check + let symbols_db_path = crate::indexer::symbol_db_path(path); + let symbols_valid = if symbols_db_path.exists() { + match SymbolStore::open(&symbols_db_path) { + Ok(_) => true, + Err(e) => { + issues.push(VerifyIssue { + component: "symbols".to_string(), + severity: VerifySeverity::Warning, + message: format!("Failed to open symbols.db: {e}"), + }); + false + } + } + } else { + // symbols.db is optional + issues.push(VerifyIssue { + component: "symbols".to_string(), + severity: VerifySeverity::Warning, + message: "symbols.db not found".to_string(), + }); + false + }; + + VerifyResult { + state_valid, + tantivy_valid, + manifest_valid, + symbols_valid, + issues, + } +} + /// status コマンドのメインロジック -pub fn run(path: &Path, format: StatusFormat, writer: &mut dyn Write) -> Result<(), StatusError> { +pub fn run( + path: &Path, + format: StatusFormat, + verify: bool, + writer: &mut dyn Write, +) -> Result<(), StatusError> { if !path.is_dir() { return Err(StatusError::DirectoryNotFound(path.to_path_buf())); } - let commandindex_dir = path.join(".commandindex"); + let commandindex_dir = crate::indexer::commandindex_dir(path); if !IndexState::exists(&commandindex_dir) { return Err(StatusError::NotInitialized); @@ -196,11 +330,50 @@ pub fn run(path: &Path, format: StatusFormat, writer: &mut dyn Write) -> Result< format_size(info.index_size_bytes) ) .ok(); + + if verify { + let result = run_verify(path); + writeln!(writer).ok(); + if result.is_ok() { + writeln!(writer, "Verify: OK").ok(); + } else { + writeln!(writer, "Verify: FAILED").ok(); + for issue in &result.issues { + let severity = match issue.severity { + VerifySeverity::Error => "ERROR", + VerifySeverity::Warning => "WARNING", + }; + writeln!( + writer, + " [{severity}] {}: {}", + issue.component, issue.message + ) + .ok(); + } + } + } } StatusFormat::Json => { - let json = serde_json::to_string_pretty(&info) - .map_err(|e| StatusError::State(StateError::Json(e)))?; - writeln!(writer, "{json}").ok(); + if verify { + let verify_result = run_verify(path); + #[derive(Serialize)] + struct StatusWithVerify { + #[serde(flatten)] + info: StatusInfo, + verify: VerifyResult, + } + let combined = StatusWithVerify { + info, + verify: verify_result, + }; + let json = serde_json::to_string_pretty(&combined) + .map_err(|e| StatusError::State(StateError::Json(e)))?; + writeln!(writer, "{json}").ok(); + } else { + let json = serde_json::to_string_pretty(&info) + .map_err(|e| StatusError::State(StateError::Json(e)))?; + writeln!(writer, "{json}").ok(); + } } } diff --git a/src/indexer/mod.rs b/src/indexer/mod.rs index 7733bcf..c7f0603 100644 --- a/src/indexer/mod.rs +++ b/src/indexer/mod.rs @@ -2,6 +2,7 @@ pub mod diff; pub mod manifest; pub mod reader; pub mod schema; +pub mod snapshot; pub mod state; pub mod symbol_store; pub mod writer; diff --git a/src/indexer/snapshot.rs b/src/indexer/snapshot.rs new file mode 100644 index 0000000..85c00f7 --- /dev/null +++ b/src/indexer/snapshot.rs @@ -0,0 +1,120 @@ +use std::path::Path; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +/// Current export format version (integer increment, forward-compatible policy) +pub const EXPORT_FORMAT_VERSION: u32 = 1; + +/// File name for export metadata inside the archive +pub const EXPORT_META_FILE: &str = "export_meta.json"; + +/// Export metadata stored as the first entry in the archive +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ExportMeta { + pub export_format_version: u32, + pub commandindex_version: String, + pub git_commit_hash: Option, + pub exported_at: DateTime, +} + +impl ExportMeta { + /// Save export metadata to a file + pub fn save(&self, path: &Path) -> Result<(), std::io::Error> { + let content = serde_json::to_string_pretty(self).map_err(std::io::Error::other)?; + std::fs::write(path, content) + } + + /// Load export metadata from a file + pub fn load(path: &Path) -> Result { + let content = std::fs::read_to_string(path)?; + let meta: Self = serde_json::from_str(&content) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + Ok(meta) + } +} + +/// Get current git HEAD commit hash for the repository at `repo_path`. +/// +/// Returns `None` if git is not available, the directory is not a git repository, +/// or the command fails for any reason. +pub fn current_git_hash(repo_path: &Path) -> Option { + std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo_path) + .output() + .ok() + .and_then(|output| { + if output.status.success() { + String::from_utf8(output.stdout) + .ok() + .map(|s| s.trim().to_string()) + } else { + None + } + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn export_meta_roundtrip() { + let meta = ExportMeta { + export_format_version: EXPORT_FORMAT_VERSION, + commandindex_version: "0.0.5".to_string(), + git_commit_hash: Some("abc123".to_string()), + exported_at: Utc::now(), + }; + + let json = serde_json::to_string(&meta).unwrap(); + let deserialized: ExportMeta = serde_json::from_str(&json).unwrap(); + + assert_eq!(deserialized.export_format_version, EXPORT_FORMAT_VERSION); + assert_eq!(deserialized.commandindex_version, "0.0.5"); + assert_eq!(deserialized.git_commit_hash, Some("abc123".to_string())); + } + + #[test] + fn export_meta_none_git_hash() { + let meta = ExportMeta { + export_format_version: 1, + commandindex_version: "0.0.5".to_string(), + git_commit_hash: None, + exported_at: Utc::now(), + }; + + let json = serde_json::to_string(&meta).unwrap(); + let deserialized: ExportMeta = serde_json::from_str(&json).unwrap(); + assert!(deserialized.git_commit_hash.is_none()); + } + + #[test] + fn export_meta_deny_unknown_fields() { + let json = r#"{"export_format_version":1,"commandindex_version":"0.0.5","git_commit_hash":null,"exported_at":"2024-01-01T00:00:00Z","unknown_field":"value"}"#; + let result: Result = serde_json::from_str(json); + assert!(result.is_err()); + } + + #[test] + fn export_meta_save_and_load() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("export_meta.json"); + + let meta = ExportMeta { + export_format_version: EXPORT_FORMAT_VERSION, + commandindex_version: "0.0.5".to_string(), + git_commit_hash: Some("def456".to_string()), + exported_at: Utc::now(), + }; + + meta.save(&path).unwrap(); + let loaded = ExportMeta::load(&path).unwrap(); + + assert_eq!(loaded.export_format_version, meta.export_format_version); + assert_eq!(loaded.commandindex_version, meta.commandindex_version); + assert_eq!(loaded.git_commit_hash, meta.git_commit_hash); + } +} diff --git a/src/main.rs b/src/main.rs index 276be04..7c07dad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -93,6 +93,9 @@ enum Commands { /// Output format (human, json) #[arg(long, value_enum, default_value_t = commandindex::cli::status::StatusFormat::Human)] format: commandindex::cli::status::StatusFormat, + /// Verify index integrity + #[arg(long)] + verify: bool, }, /// Remove index and prepare for rebuild Clean { @@ -128,6 +131,22 @@ enum Commands { #[command(subcommand)] command: ConfigCommands, }, + /// Export index as portable tar.gz archive + Export { + /// Output file path (.tar.gz) + output: PathBuf, + /// Include embedding database + #[arg(long)] + with_embeddings: bool, + }, + /// Import index from tar.gz archive + Import { + /// Input archive file path (.tar.gz) + input: PathBuf, + /// Overwrite existing index + #[arg(long)] + force: bool, + }, } #[derive(Subcommand)] @@ -281,15 +300,17 @@ fn main() { } } } - Commands::Status { path, format } => { - match commandindex::cli::status::run(&path, format, &mut std::io::stdout()) { - Ok(()) => 0, - Err(e) => { - eprintln!("{e}"); - 1 - } + Commands::Status { + path, + format, + verify, + } => match commandindex::cli::status::run(&path, format, verify, &mut std::io::stdout()) { + Ok(()) => 0, + Err(e) => { + eprintln!("{e}"); + 1 } - } + }, Commands::Context { files, max_files, @@ -357,6 +378,53 @@ fn main() { } }, }, + Commands::Export { + output, + with_embeddings, + } => { + let options = commandindex::cli::export::ExportOptions { with_embeddings }; + match commandindex::cli::export::run(std::path::Path::new("."), &output, &options) { + Ok(result) => { + println!("Export completed:"); + println!(" Output: {}", result.output_path.display()); + println!( + " Size: {}", + commandindex::cli::status::format_size(result.archive_size) + ); + if let Some(hash) = &result.git_commit_hash { + println!(" Git commit: {hash}"); + } + 0 + } + Err(e) => { + eprintln!("Error: {e}"); + 1 + } + } + } + Commands::Import { input, force } => { + let options = commandindex::cli::import_index::ImportOptions { force }; + match commandindex::cli::import_index::run(std::path::Path::new("."), &input, &options) + { + Ok(result) => { + println!("Import completed:"); + println!(" Imported files: {}", result.imported_files); + if result.git_hash_match { + println!(" Git commit: matches"); + } else { + println!(" Git commit: mismatch"); + } + for warning in &result.warnings { + println!(" Warning: {warning}"); + } + 0 + } + Err(e) => { + eprintln!("Error: {e}"); + 1 + } + } + } }; process::exit(exit_code); diff --git a/tests/cli_args.rs b/tests/cli_args.rs index 8bc7d73..3ed40bf 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -15,7 +15,9 @@ fn help_flag_shows_usage() { .stdout(predicate::str::contains("status")) .stdout(predicate::str::contains("clean")) .stdout(predicate::str::contains("context")) - .stdout(predicate::str::contains("config")); + .stdout(predicate::str::contains("config")) + .stdout(predicate::str::contains("export")) + .stdout(predicate::str::contains("import")); } #[test] diff --git a/tests/cli_export.rs b/tests/cli_export.rs new file mode 100644 index 0000000..6843328 --- /dev/null +++ b/tests/cli_export.rs @@ -0,0 +1,165 @@ +mod common; + +use std::collections::HashSet; +use std::io::Read; + +use flate2::read::GzDecoder; +use tar::Archive; + +/// Helper: create an index and return the temp dir +fn setup_indexed_dir() -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Hello\n\nWorld\n").unwrap(); + common::run_index(dir.path()); + dir +} + +/// Helper: list file names in a tar.gz archive +fn list_archive_entries(archive_path: &std::path::Path) -> HashSet { + let file = std::fs::File::open(archive_path).unwrap(); + let decoder = GzDecoder::new(file); + let mut archive = Archive::new(decoder); + let mut names = HashSet::new(); + for entry in archive.entries().unwrap() { + let entry = entry.unwrap(); + let path = entry.path().unwrap().to_string_lossy().to_string(); + names.insert(path); + } + names +} + +#[test] +fn export_basic() { + let dir = setup_indexed_dir(); + let output = dir.path().join("snapshot.tar.gz"); + + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + let result = commandindex::cli::export::run(dir.path(), &output, &options); + assert!(result.is_ok(), "export should succeed: {:?}", result.err()); + + let result = result.unwrap(); + assert!(result.output_path.exists()); + assert!(result.archive_size > 0); + + // Verify archive contents + let entries = list_archive_entries(&output); + assert!( + entries.contains("export_meta.json"), + "should contain export_meta.json" + ); + assert!(entries.contains("state.json"), "should contain state.json"); +} + +#[test] +fn export_not_initialized() { + let dir = tempfile::tempdir().expect("create temp dir"); + let output = dir.path().join("snapshot.tar.gz"); + + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + let result = commandindex::cli::export::run(dir.path(), &output, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("not initialized")); +} + +#[test] +fn export_excludes_config_local_toml() { + let dir = setup_indexed_dir(); + + // Create a config.local.toml inside .commandindex + let ci_dir = dir.path().join(".commandindex"); + std::fs::write(ci_dir.join("config.local.toml"), "secret = 'value'\n").unwrap(); + + let output = dir.path().join("snapshot.tar.gz"); + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + commandindex::cli::export::run(dir.path(), &output, &options).unwrap(); + + let entries = list_archive_entries(&output); + assert!( + !entries.contains("config.local.toml"), + "config.local.toml should be excluded" + ); +} + +#[test] +fn export_excludes_embeddings_by_default() { + let dir = setup_indexed_dir(); + + // Create a fake embeddings.db inside .commandindex + let ci_dir = dir.path().join(".commandindex"); + std::fs::write(ci_dir.join("embeddings.db"), "fake-embeddings").unwrap(); + + let output = dir.path().join("snapshot.tar.gz"); + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + commandindex::cli::export::run(dir.path(), &output, &options).unwrap(); + + let entries = list_archive_entries(&output); + assert!( + !entries.contains("embeddings.db"), + "embeddings.db should be excluded by default" + ); +} + +#[test] +fn export_includes_embeddings_when_requested() { + let dir = setup_indexed_dir(); + + // Create a fake embeddings.db inside .commandindex + let ci_dir = dir.path().join(".commandindex"); + std::fs::write(ci_dir.join("embeddings.db"), "fake-embeddings").unwrap(); + + let output = dir.path().join("snapshot.tar.gz"); + let options = commandindex::cli::export::ExportOptions { + with_embeddings: true, + }; + commandindex::cli::export::run(dir.path(), &output, &options).unwrap(); + + let entries = list_archive_entries(&output); + assert!( + entries.contains("embeddings.db"), + "embeddings.db should be included with --with-embeddings" + ); +} + +#[test] +fn export_sanitizes_index_root() { + let dir = setup_indexed_dir(); + let output = dir.path().join("snapshot.tar.gz"); + + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + commandindex::cli::export::run(dir.path(), &output, &options).unwrap(); + + // Read state.json from the archive + let file = std::fs::File::open(&output).unwrap(); + let decoder = GzDecoder::new(file); + let mut archive = Archive::new(decoder); + + for entry in archive.entries().unwrap() { + let mut entry = entry.unwrap(); + let path = entry.path().unwrap().to_string_lossy().to_string(); + if path == "state.json" { + let mut content = String::new(); + entry.read_to_string(&mut content).unwrap(); + assert!( + content.contains("__COMMANDINDEX_EXPORT_PLACEHOLDER__"), + "state.json should have sanitized index_root" + ); + assert!( + !content.contains(dir.path().to_str().unwrap()), + "state.json should not contain original path" + ); + return; + } + } + panic!("state.json not found in archive"); +} diff --git a/tests/cli_import.rs b/tests/cli_import.rs new file mode 100644 index 0000000..c69f40a --- /dev/null +++ b/tests/cli_import.rs @@ -0,0 +1,323 @@ +mod common; + +use std::io::Write; + +use flate2::Compression; +use flate2::write::GzEncoder; +use tar::Builder; + +/// Helper: create an index, export it, and return (source_dir, archive_path) +fn setup_exported_archive() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Hello\n\nWorld\n").unwrap(); + common::run_index(dir.path()); + + let archive_path = dir.path().join("snapshot.tar.gz"); + let options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + commandindex::cli::export::run(dir.path(), &archive_path, &options).unwrap(); + (dir, archive_path) +} + +/// Helper: add a bytes entry to a tar builder +fn add_tar_entry(builder: &mut Builder, name: &str, data: &[u8]) { + let mut header = tar::Header::new_gnu(); + header.set_size(data.len() as u64); + header.set_mode(0o644); + header.set_mtime(0); + header.set_cksum(); + builder.append_data(&mut header, name, data).unwrap(); +} + +/// Helper: add a bytes entry with a raw path (bypasses tar safety for malicious paths) +fn add_tar_entry_raw(builder: &mut Builder, name: &[u8], data: &[u8]) { + let mut header = tar::Header::new_gnu(); + header.set_size(data.len() as u64); + header.set_mode(0o644); + header.set_mtime(0); + header.set_entry_type(tar::EntryType::Regular); + // Write the path directly into the header name field + { + let header_bytes = header.as_old_mut(); + let name_field = &mut header_bytes.name; + let len = name.len().min(name_field.len()); + name_field[..len].copy_from_slice(&name[..len]); + if len < name_field.len() { + name_field[len..].fill(0); + } + } + header.set_cksum(); + builder.append(&header, std::io::Cursor::new(data)).unwrap(); +} + +#[test] +fn import_basic() { + let (source_dir, archive_path) = setup_exported_archive(); + + // Clean existing index + common::run_clean(source_dir.path()); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(source_dir.path(), &archive_path, &options); + assert!(result.is_ok(), "import should succeed: {:?}", result.err()); + + let result = result.unwrap(); + assert!(result.imported_files > 0); + + // Verify index is usable (state exists) + let ci_dir = source_dir.path().join(".commandindex"); + assert!(ci_dir.join("state.json").exists()); +} + +#[test] +fn import_existing_index_without_force() { + let (source_dir, archive_path) = setup_exported_archive(); + + // Don't clean - existing index should cause error + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(source_dir.path(), &archive_path, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("already exists")); +} + +#[test] +fn import_existing_index_with_force() { + let (source_dir, archive_path) = setup_exported_archive(); + + // With force, should overwrite + let options = commandindex::cli::import_index::ImportOptions { force: true }; + let result = commandindex::cli::import_index::run(source_dir.path(), &archive_path, &options); + assert!( + result.is_ok(), + "import --force should succeed: {:?}", + result.err() + ); +} + +#[test] +fn import_archive_not_found() { + let dir = tempfile::tempdir().expect("create temp dir"); + let fake_archive = dir.path().join("nonexistent.tar.gz"); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(dir.path(), &fake_archive, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("not found") || err_msg.contains("Archive")); +} + +#[test] +fn import_rejects_path_traversal_parent_dir() { + let dir = tempfile::tempdir().expect("create temp dir"); + let archive_path = dir.path().join("malicious.tar.gz"); + + // Create a malicious archive with ../escape path + let file = std::fs::File::create(&archive_path).unwrap(); + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + // Add valid export_meta.json + let meta = serde_json::json!({ + "export_format_version": 1, + "commandindex_version": "0.0.5", + "git_commit_hash": null, + "exported_at": "2024-01-01T00:00:00Z" + }); + add_tar_entry( + &mut builder, + "export_meta.json", + meta.to_string().as_bytes(), + ); + + // Add malicious path (using raw to bypass tar crate safety check) + add_tar_entry_raw(&mut builder, b"../../../etc/passwd", b"malicious content"); + + let encoder = builder.into_inner().unwrap(); + encoder.finish().unwrap(); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(dir.path(), &archive_path, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("Path traversal") || err_msg.contains("parent dir"), + "Expected path traversal error, got: {err_msg}" + ); +} + +#[test] +fn import_rejects_symlink_entry() { + let dir = tempfile::tempdir().expect("create temp dir"); + let archive_path = dir.path().join("symlink.tar.gz"); + + // Create archive with a symlink entry + let file = std::fs::File::create(&archive_path).unwrap(); + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + // Add export_meta.json + let meta = serde_json::json!({ + "export_format_version": 1, + "commandindex_version": "0.0.5", + "git_commit_hash": null, + "exported_at": "2024-01-01T00:00:00Z" + }); + add_tar_entry( + &mut builder, + "export_meta.json", + meta.to_string().as_bytes(), + ); + + // Add a symlink entry + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Symlink); + header.set_size(0); + header.set_mode(0o777); + header.set_mtime(0); + header.set_cksum(); + builder + .append_link(&mut header, "evil_link", "/etc/passwd") + .unwrap(); + + let encoder = builder.into_inner().unwrap(); + encoder.finish().unwrap(); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(dir.path(), &archive_path, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("Symlink") || err_msg.contains("symlink"), + "Expected symlink error, got: {err_msg}" + ); +} + +#[test] +fn import_rejects_incompatible_version() { + let dir = tempfile::tempdir().expect("create temp dir"); + let archive_path = dir.path().join("future_version.tar.gz"); + + // Create archive with future format version + let file = std::fs::File::create(&archive_path).unwrap(); + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + let meta = serde_json::json!({ + "export_format_version": 999, + "commandindex_version": "99.0.0", + "git_commit_hash": null, + "exported_at": "2024-01-01T00:00:00Z" + }); + add_tar_entry( + &mut builder, + "export_meta.json", + meta.to_string().as_bytes(), + ); + + let encoder = builder.into_inner().unwrap(); + encoder.finish().unwrap(); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(dir.path(), &archive_path, &options); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("Incompatible") || err_msg.contains("version"), + "Expected version error, got: {err_msg}" + ); +} + +#[test] +fn import_git_commit_hash_mismatch_shows_warning() { + let dir = tempfile::tempdir().expect("create temp dir"); + let archive_path = dir.path().join("hash_mismatch.tar.gz"); + + // Create an archive with a known (fake) git_commit_hash + let file = std::fs::File::create(&archive_path).unwrap(); + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + let meta = serde_json::json!({ + "export_format_version": 1, + "commandindex_version": "0.0.5", + "git_commit_hash": "aaaa0000bbbb1111cccc2222dddd3333eeee4444", + "exported_at": "2024-01-01T00:00:00Z" + }); + add_tar_entry( + &mut builder, + "export_meta.json", + meta.to_string().as_bytes(), + ); + + // Add a minimal state.json + let state = serde_json::json!({ + "version": "0.0.5", + "schema_version": 2, + "index_root": "__COMMANDINDEX_EXPORT_PLACEHOLDER__", + "created_at": "2024-01-01T00:00:00Z", + "last_updated_at": "2024-01-01T00:00:00Z", + "total_files": 0, + "total_sections": 0 + }); + add_tar_entry(&mut builder, "state.json", state.to_string().as_bytes()); + + let encoder = builder.into_inner().unwrap(); + encoder.finish().unwrap(); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(dir.path(), &archive_path, &options); + assert!(result.is_ok(), "import should succeed: {:?}", result.err()); + + let result = result.unwrap(); + // The current git hash won't match the fake hash + assert!(!result.git_hash_match, "git_hash_match should be false"); + assert!( + !result.warnings.is_empty(), + "should have warnings about hash mismatch" + ); + // Check that the warning mentions the hash or mismatch + let has_relevant_warning = result + .warnings + .iter() + .any(|w| w.contains("mismatch") || w.contains("commit") || w.contains("hash")); + assert!( + has_relevant_warning, + "Expected warning about commit hash, got: {:?}", + result.warnings + ); +} + +#[test] +fn import_state_json_index_root_rewritten() { + let (_source_dir, archive_path) = setup_exported_archive(); + + // Import into a different directory + let import_dir = tempfile::tempdir().expect("create import dir"); + + let options = commandindex::cli::import_index::ImportOptions { force: false }; + let result = commandindex::cli::import_index::run(import_dir.path(), &archive_path, &options); + assert!(result.is_ok(), "import should succeed: {:?}", result.err()); + + // Read state.json and verify index_root was rewritten + let ci_dir = import_dir.path().join(".commandindex"); + let state_content = std::fs::read_to_string(ci_dir.join("state.json")).unwrap(); + let state: serde_json::Value = serde_json::from_str(&state_content).unwrap(); + + let index_root = state["index_root"].as_str().unwrap(); + assert!( + !index_root.contains("PLACEHOLDER"), + "index_root should not contain placeholder" + ); + + // Allow both original path and import path (depends on canonicalization) + // The key thing is that it shouldn't contain the placeholder + assert!(!index_root.is_empty(), "index_root should not be empty"); + + // Verify source dir's original path is NOT in the state + assert!( + !index_root.contains("__COMMANDINDEX_EXPORT_PLACEHOLDER__"), + "Should not contain placeholder" + ); +} diff --git a/tests/cli_status.rs b/tests/cli_status.rs index 882bbb4..25cf289 100644 --- a/tests/cli_status.rs +++ b/tests/cli_status.rs @@ -67,7 +67,7 @@ fn compute_dir_size_nested() { fn run_directory_not_found() { let mut buf = Cursor::new(Vec::new()); let path = PathBuf::from("/nonexistent/path/that/does/not/exist"); - let result = run(&path, StatusFormat::Human, &mut buf); + let result = run(&path, StatusFormat::Human, false, &mut buf); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("Directory not found")); @@ -77,7 +77,7 @@ fn run_directory_not_found() { fn run_not_initialized() { let dir = tempfile::tempdir().expect("create temp dir"); let mut buf = Cursor::new(Vec::new()); - let result = run(dir.path(), StatusFormat::Human, &mut buf); + let result = run(dir.path(), StatusFormat::Human, false, &mut buf); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("not initialized")); @@ -99,7 +99,7 @@ fn run_human_format() { setup_commandindex_dir(dir.path()); let mut buf = Cursor::new(Vec::new()); - run(dir.path(), StatusFormat::Human, &mut buf).expect("run should succeed"); + run(dir.path(), StatusFormat::Human, false, &mut buf).expect("run should succeed"); let output = String::from_utf8(buf.into_inner()).expect("valid utf8"); assert!(output.contains("CommandIndex Status")); @@ -118,7 +118,7 @@ fn run_json_format() { setup_commandindex_dir(dir.path()); let mut buf = Cursor::new(Vec::new()); - run(dir.path(), StatusFormat::Json, &mut buf).expect("run should succeed"); + run(dir.path(), StatusFormat::Json, false, &mut buf).expect("run should succeed"); let output = String::from_utf8(buf.into_inner()).expect("valid utf8"); let parsed: serde_json::Value = serde_json::from_str(&output).expect("valid json"); diff --git a/tests/e2e_export_import.rs b/tests/e2e_export_import.rs new file mode 100644 index 0000000..feb2844 --- /dev/null +++ b/tests/e2e_export_import.rs @@ -0,0 +1,107 @@ +mod common; + +#[test] +fn e2e_export_import_search() { + // 1. Create source with markdown content + let source_dir = tempfile::tempdir().expect("create source dir"); + std::fs::write( + source_dir.path().join("readme.md"), + "# Project Overview\n\nThis is a test project for export import flow.\n", + ) + .unwrap(); + + // 2. Index the source + common::run_index(source_dir.path()); + + // 3. Verify search works on source + let results = common::run_search_jsonl(source_dir.path(), "test project"); + assert!(!results.is_empty(), "search should find results in source"); + + // 4. Export + let archive_path = source_dir.path().join("index-snapshot.tar.gz"); + let export_options = commandindex::cli::export::ExportOptions { + with_embeddings: false, + }; + let export_result = + commandindex::cli::export::run(source_dir.path(), &archive_path, &export_options).unwrap(); + assert!(export_result.archive_size > 0); + + // 5. Import into a new directory + let import_dir = tempfile::tempdir().expect("create import dir"); + // Copy the markdown file to the import dir (so relative paths work) + std::fs::write( + import_dir.path().join("readme.md"), + "# Project Overview\n\nThis is a test project for export import flow.\n", + ) + .unwrap(); + + let import_options = commandindex::cli::import_index::ImportOptions { force: false }; + let import_result = + commandindex::cli::import_index::run(import_dir.path(), &archive_path, &import_options) + .unwrap(); + assert!(import_result.imported_files > 0); + + // 6. Verify search works on imported index + let results = common::run_search_jsonl(import_dir.path(), "test project"); + assert!( + !results.is_empty(), + "search should find results after import" + ); + + // 7. Verify status works on imported index + let status = common::run_status_json(import_dir.path()); + assert!( + status.get("version").is_some(), + "status should show version after import" + ); +} + +#[test] +fn e2e_export_import_cli() { + // Test via CLI binary + let source_dir = tempfile::tempdir().expect("create source dir"); + std::fs::write( + source_dir.path().join("doc.md"), + "# Documentation\n\nSome content here.\n", + ) + .unwrap(); + + // Index + common::run_index(source_dir.path()); + + // Export via CLI + let archive_path = source_dir.path().join("export.tar.gz"); + common::cmd() + .args(["export", archive_path.to_str().unwrap()]) + .current_dir(source_dir.path()) + .assert() + .success(); + + assert!(archive_path.exists(), "archive should be created"); + + // Import via CLI into new dir + let import_dir = tempfile::tempdir().expect("create import dir"); + std::fs::write( + import_dir.path().join("doc.md"), + "# Documentation\n\nSome content here.\n", + ) + .unwrap(); + + common::cmd() + .args(["import", archive_path.to_str().unwrap()]) + .current_dir(import_dir.path()) + .assert() + .success(); + + // Verify status works + common::cmd() + .args([ + "status", + "--path", + import_dir.path().to_str().unwrap(), + "--format", + "json", + ]) + .assert() + .success(); +} diff --git a/tests/e2e_verify.rs b/tests/e2e_verify.rs new file mode 100644 index 0000000..15f7732 --- /dev/null +++ b/tests/e2e_verify.rs @@ -0,0 +1,77 @@ +mod common; + +use predicates::prelude::*; + +#[test] +fn verify_normal_index() { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Test\n\nContent\n").unwrap(); + common::run_index(dir.path()); + + common::cmd() + .args(["status", "--path", dir.path().to_str().unwrap(), "--verify"]) + .assert() + .success() + .stdout(predicate::str::contains("Verify: OK")); +} + +#[test] +fn verify_corrupted_index() { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Test\n\nContent\n").unwrap(); + common::run_index(dir.path()); + + // Corrupt tantivy directory by removing it + let tantivy_dir = dir.path().join(".commandindex").join("tantivy"); + std::fs::remove_dir_all(&tantivy_dir).unwrap(); + + common::cmd() + .args(["status", "--path", dir.path().to_str().unwrap(), "--verify"]) + .assert() + .success() + .stdout(predicate::str::contains("Verify: FAILED")); +} + +#[test] +fn verify_json_format() { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Test\n\nContent\n").unwrap(); + common::run_index(dir.path()); + + let output = common::cmd() + .args([ + "status", + "--path", + dir.path().to_str().unwrap(), + "--format", + "json", + "--verify", + ]) + .assert() + .success(); + + let stdout = String::from_utf8_lossy(&output.get_output().stdout); + let parsed: serde_json::Value = serde_json::from_str(&stdout).expect("valid json"); + assert!(parsed.get("verify").is_some(), "should contain verify key"); + assert!( + parsed["verify"]["state_valid"].as_bool().unwrap(), + "state should be valid" + ); + assert!( + parsed["verify"]["tantivy_valid"].as_bool().unwrap(), + "tantivy should be valid" + ); +} + +#[test] +fn verify_without_flag_no_verify_output() { + let dir = tempfile::tempdir().expect("create temp dir"); + std::fs::write(dir.path().join("test.md"), "# Test\n\nContent\n").unwrap(); + common::run_index(dir.path()); + + common::cmd() + .args(["status", "--path", dir.path().to_str().unwrap()]) + .assert() + .success() + .stdout(predicate::str::contains("Verify:").not()); +}