diff --git a/AGENTS.md b/AGENTS.md index 7c2d4f6..60aaab6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -227,7 +227,7 @@ Provider state file note: - Cooldown keys are composite, not provider-name-only: `ProviderStep::cooldown_key()` returns the explicit `name` (lowercased) if set, otherwise a deterministic key derived from canonical provider + model + env + command, joined by US (0x1F) so model names with spaces/parentheses/colons and env values with slashes never collide. This is what makes "the same provider on a different model or account is demoted independently" hold (e.g. `codex` on account A can be in cooldown while `codex` on account B keeps working, and `antigravity` on the Gemini model stays usable when the GPT-OSS step is cooling down). `State.failures` is a `Vec` (was a `HashMap`); `State::load` migrates an old provider-name-keyed file by mapping each legacy key through the "provider-only step" `cooldown_key`, so existing cooldowns keep applying and `gemini`/`apple-ai` legacy keys still merge into `antigravity`/`apple-intelligence`. The legacy in-memory `migrate_legacy_gemini_key` is gone — `canonical_provider_key` (now in `config.rs`, shared by `ProviderStep::cooldown_key` and the state migration) handles the alias merge. Developer generation log note: -- `[dev_log] enabled = true` (global config only) records one JSON file per run under `~/.config/git-sc/logs/YYYY-MM-DD/`, so prompt changes can be evaluated against measured failure rates instead of the after-the-fact `git log` scans that every earlier prompt fix in this file relied on. `DevLog` (`devlog.rs`) is built in `App::new`, shared with `AiService` as an `Rc`, and written exactly once from `App::run` — routing every mode through a single exit is what keeps a new operation mode from silently skipping the log. A run that never reached generation writes nothing (`finish` returns early when no prompt was set), because the interesting unit is a prompt/response pair, not a process start. +- `[dev_log] enabled = true` (global config only) records one JSON file per run under `~/.config/git-sc/logs/YYYY-MM-DD/`, so prompt changes can be evaluated against measured failure rates instead of the after-the-fact `git log` scans that every earlier prompt fix in this file relied on. `DevLog` (`devlog.rs`) is built in `App::new`, shared with `AiService` as an `Rc`, and written exactly once from `App::run` — routing every mode through a single exit is what keeps a new operation mode from silently skipping the log. A successful run that never reached generation writes nothing. Failures before generation (including staging and Git startup errors) are recorded with `prompt: null`, `result.status: "failed"`, and `result.error`. `App::run` captures repository verification errors in the same finalization path; `NotGitRepository` remains an unlogged normal skip because the CLI treats it as exit 0. Nullable prompt metadata keeps schema version 1. - **One file per run, published by rename.** git-sc is a short-lived process that runs concurrently across repositories, and a record carrying a full prompt is tens of KB. A shared daily JSONL cannot guarantee non-interleaved lines at that size — `O_APPEND` on a regular file only serializes where the write starts, not that a large write lands in one piece — so the choice is either an inter-process lock or per-run files. Per-run files are simpler and also make a partially written record impossible to mistake for a finished one: `write_record` writes `.{run_id}.tmp` with `create_new(true)` + mode `0600`, then `rename`s it into place (the same technique as `State::save`). Analysis converts them back with `find … -name '*.json' | xargs jq -c .`. - **`content = "metadata"` (default) must not leak the diff, and that takes two exclusions, not one.** Omitting the prompt is the obvious half. The other is provider **stderr**: Codex echoes the prompt to stderr (`Reading prompt from stdin...` followed by the whole thing), so capturing stderr at the metadata level puts the staged diff back in the log through the side door. Caught by inspecting a real run's record on 2026-09-02 (JST) — the field held 2.7 KB of prompt echo — so `call_provider` now drops the stderr body entirely at that level and keeps only `stderr_bytes`. The one-line reason a provider failed still survives in `error` (from `extract_error`), which is what failure analysis actually needs. Raw **stdout** is kept at both levels on purpose: `fix: x` and `fix: x` clean up to the same string, so without the raw response a wrong-tag or truncation event is indistinguishable from a normal one after the fact. - Other records are deliberately non-secret: `env` overrides are logged by key name only (values are `CODEX_HOME`-style paths but nothing stops a credential from being there), and `provider_plan` uses `step_plan_label` — provider + configured model, or an explicit `name` — rather than `cooldown_key`, which embeds env **values**. Files land mode `0600` inside `0700` directories. diff --git a/docs/configuration.ja.md b/docs/configuration.ja.md index 6f8f3c1..04d1609 100644 --- a/docs/configuration.ja.md +++ b/docs/configuration.ja.md @@ -240,6 +240,8 @@ max_total_mb = 500 1 回の実行につき 1 つの JSON ファイルを `~/.config/git-sc/logs/YYYY-MM-DD/` に書き出します。記録する内容は、プロンプトのハッシュと差分の統計、各プロバイダーの試行、実行の結末です。試行ごとに整形前の生応答・モデル・所要時間・品質判定と、採用/引き直し/フォールバックのどれだったかを残し、結末にはコミットした場合のハッシュも含めます。実行ごとに別のファイルに書くので、`git-sc` を同時に走らせても記録が混ざりません。また、一時ファイルに書き切ってから rename して公開するため、書きかけのファイルが完成品として解析対象に紛れ込むこともありません。 +ステージング失敗など、プロンプト作成前の失敗も `result.status = "failed"`、`result.error` にエラー、`prompt = null` として記録します。変更がない場合の `--all` やリポジトリ外での起動など、生成せず正常にスキップした実行は記録しません。ログ収集は設定読み込み後に始まるため、設定読み込み自体のエラーは記録対象外です。 + ファイル群は、そのまま JSONL に流し込んで解析できます: ```bash diff --git a/docs/configuration.md b/docs/configuration.md index 767ecab..31b4706 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -240,6 +240,8 @@ max_total_mb = 500 Each run writes one JSON file to `~/.config/git-sc/logs/YYYY-MM-DD/`, containing the prompt digest and diff statistics, every provider attempt (raw response before cleanup, model, duration, quality findings, and whether it was accepted, retried, or fell through), and the outcome — including the commit hash when one was made. Files are written to a temporary name and renamed into place, so concurrent `git-sc` runs never interleave, and partially written records never appear as finished ones. +Failures before prompt construction, such as staging failures, are also recorded with `result.status = "failed"`, the error in `result.error`, and `prompt = null`. Successful runs that skip generation (for example, `--all` with no changes or invocation outside a repository) do not create a log. Logging begins after configuration has loaded, so configuration-loading errors are not recorded. + Analyze them by streaming the files into JSONL: ```bash diff --git a/src/app.rs b/src/app.rs index 3d2a860..4c7d718 100644 --- a/src/app.rs +++ b/src/app.rs @@ -711,9 +711,6 @@ impl App { // claw-hooks stop hook から渡されるエージェントコンテキスト let agent_context = std::env::var("CLAW_HOOKS_AGENT_MESSAGE").ok(); - // Gitリポジトリかどうかを確認 - self.git.verify_repository()?; - if let Some(dev_log) = &self.dev_log { dev_log.set_invocation(Invocation { mode: Self::run_mode(cli).to_string(), @@ -724,23 +721,32 @@ impl App { stage_all: cli.stage_all, from_agent_hook: agent_context.is_some(), }); - dev_log.set_repository(Repository { - path: self - .git - .get_git_root() - .map(|root| root.to_string_lossy().into_owned()), - branch: self.git.get_current_branch(), - }); } - let result = match self.dispatch_special_mode(cli, agent_context.as_deref()) { - Some(result) => result, - None => self.run_commit(cli, agent_context.as_deref()), - }; + // 検証段階のエラーも共通の終了処理へ流す。 + let result = self.git.verify_repository().and_then(|()| { + if let Some(dev_log) = &self.dev_log { + dev_log.set_repository(Repository { + path: self + .git + .get_git_root() + .map(|root| root.to_string_lossy().into_owned()), + branch: self.git.get_current_branch(), + }); + } + + match self.dispatch_special_mode(cli, agent_context.as_deref()) { + Some(result) => result, + None => self.run_commit(cli, agent_context.as_deref()), + } + }); // 生成ログはここで 1 度だけ書き出す。各モードの戻り先を 1 箇所に絞ることで、 // 出口を増やしても記録漏れが起きないようにする。 - if let Some(dev_log) = &self.dev_log { + // リポジトリ外での起動は main が exit 0 として扱う通常スキップ。 + if let Some(dev_log) = &self.dev_log + && !matches!(result, Err(AppError::NotGitRepository)) + { dev_log.finish(result.as_ref().err().map(|e| e.to_string())); } diff --git a/src/devlog.rs b/src/devlog.rs index f73e48a..cede56c 100644 --- a/src/devlog.rs +++ b/src/devlog.rs @@ -144,7 +144,7 @@ struct RunRecord { invocation: Invocation, repository: Repository, input: Input, - prompt: PromptInfo, + prompt: Option, provider_plan: Vec, attempts: Vec, result: RunResult, @@ -350,25 +350,24 @@ impl DevLog { /// 実行結果を書き出す /// - /// 生成に至らなかった実行(ステージ済みの変更が無い等)は記録しない。改善パイプラインで - /// 見たいのは「プロンプトと応答の対」であって、起動回数ではないため。 + /// 生成に至らなかった正常なスキップは記録しない。失敗は生成前でも記録し、 + /// その場合は prompt を null として生成した実行と区別する。 /// /// 書き込みに失敗してもエラーは返さない。ログのために生成やコミットを /// 止めるのは本末転倒なので、警告を 1 行出して続行する。 pub fn finish(&self, error: Option) { let run = self.run.borrow(); - let Some(prompt) = run.prompt.as_deref() else { - return; - }; - let mut result = run.result.clone(); - if result.status.is_empty() { - // set_result まで到達しなかった = 途中で失敗した実行 - result.status = "failed".to_string(); - } if result.error.is_none() { result.error = error; } + if run.prompt.is_none() && result.error.is_none() && result.status != "failed" { + return; + } + if result.error.is_some() || result.status.is_empty() { + // set_result まで到達しなかった、または記録後に失敗した実行 + result.status = "failed".to_string(); + } let run_id = Self::new_run_id(self.started_at_unix_ms); let record = RunRecord { @@ -382,14 +381,14 @@ impl DevLog { invocation: run.invocation.clone(), repository: run.repository.clone(), input: run.input.clone(), - prompt: PromptInfo { + prompt: run.prompt.as_deref().map(|prompt| PromptInfo { bytes: prompt.len(), digest: digest(prompt), content: match self.content_level { ContentLevel::Full => Some(prompt.to_string()), ContentLevel::Metadata => None, }, - }, + }), provider_plan: run.provider_plan.clone(), attempts: self.attempts.borrow().clone(), result, @@ -713,7 +712,7 @@ mod tests { assert_eq!(written["prompt"]["content"], "prompt body"); } - /// 生成に至らなかった実行(ステージ済みの変更が無い等)はログを残さない + /// 生成せず正常にスキップした実行はログを残さない #[test] fn test_run_without_generation_is_not_recorded() { let dir = tempfile::TempDir::new().unwrap(); @@ -743,6 +742,39 @@ mod tests { assert_eq!(written["result"]["error"], "all providers failed"); } + /// プロンプトを作る前の失敗も、生成情報なしでエラーを残す + #[test] + fn test_failed_run_before_generation_records_error() { + for content in ["metadata", "full"] { + let dir = tempfile::TempDir::new().unwrap(); + let log = DevLog::from_config(&enabled_config(dir.path(), content), true).unwrap(); + log.finish(Some("staging failed".to_string())); + + let written = read_single_record(dir.path()); + assert_eq!(written["result"]["status"], "failed"); + assert_eq!(written["result"]["error"], "staging failed"); + assert_eq!(written.get("prompt"), Some(&serde_json::Value::Null)); + assert!(written["attempts"].as_array().unwrap().is_empty()); + } + } + + #[test] + fn test_explicit_failure_before_generation_records_error() { + let dir = tempfile::TempDir::new().unwrap(); + let log = DevLog::from_config(&enabled_config(dir.path(), "metadata"), true).unwrap(); + log.set_result(RunResult { + status: "failed".to_string(), + error: Some("staging failed".to_string()), + ..RunResult::default() + }); + log.finish(None); + + let written = read_single_record(dir.path()); + assert_eq!(written["result"]["status"], "failed"); + assert_eq!(written["result"]["error"], "staging failed"); + assert_eq!(written.get("prompt"), Some(&serde_json::Value::Null)); + } + #[test] fn test_records_attempts_in_order() { let dir = tempfile::TempDir::new().unwrap(); diff --git a/src/git/service.rs b/src/git/service.rs index 28fb990..abcf125 100644 --- a/src/git/service.rs +++ b/src/git/service.rs @@ -1842,11 +1842,13 @@ index 1234567..abcdefg 100644 #[test] fn test_get_current_branch() { - let service = GitService::new(); - let branch = service.get_current_branch(); - // ブランチ名が取得できること（空でないこと） - assert!(branch.is_some()); - assert!(!branch.unwrap().is_empty()); + let temp_dir = setup_temp_git_repo(); + let repo = temp_dir.path(); + run_git_in(repo, &["symbolic-ref", "HEAD", "refs/heads/test-branch"]); + run_git_in(repo, &["commit", "--allow-empty", "-m", "initial"]); + let service = GitService::with_repo_path(repo.to_path_buf()); + + assert_eq!(service.get_current_branch().as_deref(), Some("test-branch")); } #[test] @@ -1896,11 +1898,13 @@ index 1234567..abcdefg 100644 #[test] fn test_branch_exists_main() { - let service = GitService::new(); - // main または master ブランチが存在するはず - let main_exists = service.branch_exists("main"); - let master_exists = service.branch_exists("master"); - assert!(main_exists || master_exists); + let temp_dir = setup_temp_git_repo(); + let repo = temp_dir.path(); + run_git_in(repo, &["symbolic-ref", "HEAD", "refs/heads/main"]); + run_git_in(repo, &["commit", "--allow-empty", "-m", "initial"]); + let service = GitService::with_repo_path(repo.to_path_buf()); + + assert!(service.branch_exists("main")); } #[test] diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 75335f7..5a5477c 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -617,6 +617,108 @@ fn test_run_outside_git_repo() { // --all で変更がない場合のテスト // ============================================================ +/// 生成前の失敗が 1 実行 1 ファイルとして残ったことを確かめる +fn read_single_dev_log(config_dir: &std::path::Path) -> serde_json::Value { + let logs: Vec<_> = std::fs::read_dir(config_dir.join("logs")) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.is_dir()) + .flat_map(|day| std::fs::read_dir(day).unwrap()) + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "json")) + .collect(); + assert_eq!(logs.len(), 1, "失敗した実行のログは 1 件だけ残る"); + serde_json::from_slice(&std::fs::read(&logs[0]).unwrap()).unwrap() +} + +/// index.lock による生成前のステージング失敗も、quiet モードでログに残る +#[test] +#[cfg_attr(windows, ignore)] +fn test_dev_log_records_staging_failure_before_generation() { + let dir = setup_git_repo_with_commit(); + let home = TempDir::new().unwrap(); + let config_dir = home.path().join(".config/git-sc"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::write( + config_dir.join("config.toml"), + "nano_buddy = false\n[dev_log]\nenabled = true\n", + ) + .unwrap(); + std::fs::write(dir.path().join("README.md"), "# Updated\n").unwrap(); + std::fs::write(dir.path().join(".git/index.lock"), "").unwrap(); + + let output = git_sc!() + .args(["--all", "--yes", "--quiet"]) + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", home.path()) + .current_dir(dir.path()) + .assert() + .failure() + .stderr(predicate::str::contains("index.lock")) + .get_output() + .clone(); + assert!(output.stdout.is_empty()); + + let record = read_single_dev_log(&config_dir); + assert_eq!(record["result"]["status"], "failed"); + assert_eq!( + record["result"]["error"].as_str().unwrap(), + String::from_utf8(output.stderr) + .unwrap() + .trim() + .strip_prefix("Error: ") + .unwrap() + ); + assert_eq!(record.get("prompt"), Some(&serde_json::Value::Null)); + assert!(record["attempts"].as_array().unwrap().is_empty()); + assert_eq!(record["invocation"]["mode"], "commit"); + assert_eq!(record["invocation"]["quiet"], true); + assert_eq!(record["invocation"]["stage_all"], true); + assert_eq!(record["invocation"]["auto_confirm"], true); +} + +#[test] +#[cfg_attr(windows, ignore)] +fn test_dev_log_records_git_verification_failure_but_skips_non_repository() { + let dir = TempDir::new().unwrap(); + let home = TempDir::new().unwrap(); + let config_dir = home.path().join(".config/git-sc"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::write( + config_dir.join("config.toml"), + "nano_buddy = false\n[dev_log]\nenabled = true\n", + ) + .unwrap(); + + // リポジトリ外での実行は exit 0 の通常スキップなので記録しない。 + git_sc!() + .arg("--quiet") + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", home.path()) + .current_dir(dir.path()) + .assert() + .success(); + assert!(!config_dir.join("logs").exists()); + + // Git 自体を起動できないエラーは、検証段階でも記録する。 + let empty_path = TempDir::new().unwrap(); + git_sc!() + .arg("--quiet") + .env("PATH", empty_path.path()) + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", home.path()) + .current_dir(dir.path()) + .assert() + .failure(); + let record = read_single_dev_log(&config_dir); + assert_eq!(record["result"]["status"], "failed"); + assert!(!record["result"]["error"].as_str().unwrap().is_empty()); + assert_eq!(record.get("prompt"), Some(&serde_json::Value::Null)); + assert!(record["attempts"].as_array().unwrap().is_empty()); + assert_eq!(record["invocation"]["mode"], "commit"); + assert_eq!(record["invocation"]["quiet"], true); +} + #[test] fn test_stage_all_no_changes() { let dir = setup_git_repo_with_commit();