Skip to content

Commit 1d00933

Browse files
owayoclaude
andcommitted
fix: 履歴破壊・除外設定の無効化・情報漏洩につながる不具合を修正
レビューで見つかった、いずれも実機で再現を確認した不具合をまとめて修正する。 履歴を壊すもの: - reword が `rebase.rebaseMerges = true` の設定下で無言の no-op になっていた。 todo の先頭に `label onto` が入り 1 行目の置換が空振りするが、todo は全行 pick のまま有効なので rebase は exit 0 で完了し、「reword 成功」と表示した うえで force push まで案内していた。`--no-rebase-merges` で隔離し、さらに sequence editor 側で先頭行が pick でなければ rebase ごと失敗させる。 - squash / amend が生成中の HEAD 移動を検出していなかった。畳む・書き換える 対象は index ではなく履歴側にあるため、別端末のコミットは index を汚さず ステージ変更の確認をすり抜ける。squash では AI が見ていないコミットが 混入し、amend では説明していない別コミットを書き換えていた。差分取得前の HEAD を控え、履歴を動かす直前に突き合わせる。通常のコミット経路にも同じ 比較を追加した (index の tree が同じでも HEAD が戻れば範囲が広がるため)。 - 確認プロンプトが stdin の EOF を `[Y/n]` の既定値 (Yes) として扱っていた。 標準入力を閉じたスクリプトや hook から `--yes` なしで呼ぶと、プロンプトを 出した直後に無人で承認され amend / squash / reword が実行されていた。 除外設定・情報漏洩: - マージコミットで `.git-sc-ignore` が完全に無効化されていた。マージへの `git show` は combined diff (`diff --cc <path>`) を出すため、`diff --git` 前提のブロック検出が 1 つも成立せず、除外対象の中身がそのまま AI へ 送られていた。combined 形式のヘッダーを認識するようにする。 - 空応答時のエラーに stderr 全文を埋め込んでいた。Codex は stderr へ プロンプト全文 (= staged diff) をエコーするため、端末と生成ログの `error` に差分が流出していた。他の失敗経路と同じ 1 行抽出に揃える。 - `has_staged_changes()` に `--no-relative` が無く、`diff.relative = true` の 設定下でサブディレクトリから実行すると cwd 外のステージを見落としていた。 通常コミットが無言でスキップされ、squash の安全ガードもすり抜けていた。 コミットメッセージの品質: - 引用符の除去が両端を独立に削っていたため、`revert: "feat: X"` のように 片側だけが引用符の件名から閉じ引用符を落とし、閉じていないメッセージを 作っていた。対になっているときだけ剥がす。副作用として引用符だけの応答が 通り得るようになるため、その場合は従来どおり空応答として扱う。 その他: - 生成ログの `started_at` が終了時刻を記録していた。開始時刻の epoch との 差が常に `duration_ms` と一致することを実ログ 8 件で確認。時計を 1 回だけ 読むようにし、日付ディレクトリも開始時刻から決める。 - Windows の `--all` が cwd 配下しかステージしていなかった (pathspec を `:/` に変更)。また reword のエディタ文字列が PowerShell 版で、git が エディタを sh 経由で起動する都合により変数が展開されて機能していなかった ため、sh 版へ一本化した。 - `[ai_usage] command` の実行ファイルが `~` 展開されず、fail-open のため 残量ゲートが無言で無効になっていた。 各修正に回帰テストを追加し、いずれも修正を戻すと失敗することを確認済み。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7bf5b19 commit 1d00933

11 files changed

Lines changed: 808 additions & 44 deletions

File tree

‎AGENTS.md‎

Lines changed: 15 additions & 0 deletions
Large diffs are not rendered by default.

‎README.ja.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ git-sc -n
141141

142142
- `--reword` は rebase が進行中のときは実行を拒否します。reword は失敗した rebase を必ず `git rebase --abort` で終わらせるため、進行中の rebase の上に重ねると解決作業中の内容を破棄してしまうためです。先に rebase を完了するか中止してください。
143143
- メッセージ生成中にステージ内容が変化した場合、コミットを中止します(生成前後の index の tree を比較します)。生成済みメッセージは変更前の内容を説明したものなので、変更後の内容をそのままコミットするのは誤りだからです。そのまま再実行してください。`--squash` も reset の直前に同じ確認を行います。
144+
- 通常のコミット、`--amend`、`--squash` は、生成中に `HEAD` が動いた場合も中止します。`--squash` がまとめる対象と `--amend` が書き換える対象は index ではなく履歴側にあるため、別の端末でコミットされても index はきれいなままで、ステージ変更の確認をすり抜けます。その状態で進むと、`--squash` は AI が見ていないコミットまで巻き込み、`--amend` は説明していない別のコミットを書き換えてしまいます。
144145

145146
### オプション
146147

@@ -197,6 +198,9 @@ git-sc -n
197198
| `--help` | `-h` | ヘルプを表示 |
198199
| `--version` | `-V` | バージョンを表示 |
199200

201+
`--yes` の動作:
202+
- 無人実行では必須です。確認プロンプトで標準入力が EOF になった場合(スクリプトや hook から標準入力を閉じた状態で呼ばれた場合など)は、`[Y/n]` の既定値を採らずエラーで中止します。ユーザーが入力した空行は従来どおり「はい」ですが、「入力自体が無い」状態は別扱いです。同じプロンプトが `--amend` / `--squash` / `--reword` も守っているためです
203+
200204
`--quiet` の動作:
201205
- 通常実行 / amend / squash / reword の進捗・プレビュー・成功/キャンセル表示を抑制
202206
- エラー出力はそのまま表示
@@ -455,7 +459,7 @@ auto_push = true
455459
```toml
456460
[ai_usage]
457461
enabled = true
458-
command = ["ai-usage", "--json"] # 省略可
462+
command = ["ai-usage", "--json"] # 省略可(実行ファイルのパスの `~` は展開されます)
459463
threshold_percent = 95 # この使用率以上のステップを除外する
460464
window = "nearest" # weekly / five_hour / nearest (両者のうち高い方)
461465
timeout_seconds = 10

‎README.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ Concurrency guards worth knowing about:
141141

142142
- `--reword` refuses to run while a rebase is in progress. It ends every failed rebase with `git rebase --abort`, so starting one on top of yours would discard your in-progress conflict resolution. Finish or abort your rebase first.
143143
- Committing aborts if the staged content changed while the message was being generated (git-sc compares the index tree before and after). The generated message describes the old content, so committing the new one would be wrong. Just re-run. `--squash` re-checks for newly staged changes the same way, right before it resets.
144+
- Committing, `--amend`, and `--squash` also abort if `HEAD` moved during generation. What `--squash` folds and `--amend` rewrites comes from the history rather than the index, so a commit made in another terminal leaves the index clean and slips past the staged-changes check — a `--squash` would then fold in a commit the AI never saw, and an `--amend` would overwrite a different commit than the one it described.
144145

145146
### Options
146147

@@ -197,6 +198,9 @@ Operation modes (`--amend`, `--squash`, `--reword`, `--generate-for`) are mutual
197198
| `--help` | `-h` | Print help |
198199
| `--version` | `-V` | Print version |
199200

201+
`--yes` behavior:
202+
- Required for unattended runs. If the confirmation prompt reaches end-of-file on stdin (for example when git-sc is invoked from a script or hook with stdin closed), the run aborts with an error instead of taking the `[Y/n]` default. An empty line typed by a user still means yes; "no input at all" does not, because the same prompt guards `--amend`, `--squash`, and `--reword`
203+
200204
`--quiet` behavior:
201205
- Suppresses progress, preview, and success/cancel messages in normal/amend/squash/reword flows
202206
- Keeps error output visible
@@ -455,7 +459,7 @@ If you have the `ai-usage` CLI installed, git-sc can drop providers whose accoun
455459
```toml
456460
[ai_usage]
457461
enabled = true
458-
command = ["ai-usage", "--json"] # optional
462+
command = ["ai-usage", "--json"] # optional (`~` in the executable path is expanded)
459463
threshold_percent = 95 # skip a step at or above this usage
460464
window = "nearest" # weekly | five_hour | nearest (the higher of the two)
461465
timeout_seconds = 10

‎src/ai/process.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,16 @@ impl AiService {
360360
let message = cleaned.message;
361361

362362
if message.is_empty() {
363-
// stderr にヒントがあればそれも含める
363+
// stderr にヒントがあればそれも含める。ただし生の stderr は貼らない:
364+
// Codex は "Reading prompt from stdin..." に続けてプロンプト全文
365+
// (= staged diff)を stderr へエコーするため、全文を埋め込むと
366+
// 端末と生成ログの `error` に diff が流れ込む。`error` は詳細度
367+
// (`content = "metadata"`)の振り分け対象外なので、そこでは止まらない。
364368
if !stderr_str.trim().is_empty() {
365369
return Err(AppError::AiProviderError(format!(
366370
"{} returned an empty response (stderr: {})",
367371
provider.name(),
368-
stderr_str.trim()
372+
Self::extract_error(stderr_str, provider)
369373
)));
370374
}
371375
return Err(AppError::AiProviderError(format!(

‎src/ai/prompt.rs‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,18 +182,55 @@ Instructions:
182182
None => (Self::strip_commit_tags(&message), None),
183183
};
184184

185-
// 先頭と末尾の引用符がある場合は削除
186-
let message = message.trim_matches('"').trim_matches('\'');
185+
// 先頭と末尾が対になっている引用符だけを削除する
186+
let message = Self::strip_wrapping_quotes(&message);
187187

188188
let message = message.trim().to_string();
189189

190+
// 引用符と空白しか残らない応答は空扱いにする。対になっていない引用符を
191+
// 保持するようにしたことで、`"` だけの応答が「1 文字の有効なメッセージ」
192+
// として通ってしまうようになった(以前は両端を独立に削るので空になり、
193+
// 空応答として次のプロバイダーへ落ちていた)。中身が無い点は同じなので、
194+
// 空応答の扱いに戻す。
195+
let message = if message
196+
.chars()
197+
.all(|c| c.is_whitespace() || matches!(c, '"' | '\''))
198+
{
199+
String::new()
200+
} else {
201+
message
202+
};
203+
190204
// 件名と本文の間に空行を保証
191205
CleanedResponse {
192206
message: Self::ensure_body_separator(&message),
193207
envelope_tag,
194208
}
195209
}
196210

211+
/// 先頭と末尾が同じ引用符で「対になっている」ときだけ剥がす
212+
///
213+
/// `trim_matches` は両端を独立に削るため、片側にしか引用符が無い件名から
214+
/// その 1 つだけを落としてしまう。`revert: "feat: 認証追加"` の閉じ引用符や
215+
/// `"foo" のバグを修正` の開き引用符は本文の一部であって、囲みではない。
216+
/// 落とすと引用符が閉じていないメッセージがそのままコミットされる(下流の
217+
/// `is_truncated_subject` / `has_leftover_markup` はどちらもこれを捕まえない)。
218+
///
219+
/// 引用符の種類ごとに順に処理するのは、既存の段階的な挙動
220+
/// (`'"feat: x"'` は外側の single だけ剥がして内側の double を残す)を保つため。
221+
fn strip_wrapping_quotes(message: &str) -> &str {
222+
let mut current = message;
223+
for quote in ['"', '\''] {
224+
while let Some(inner) = current
225+
.strip_prefix(quote)
226+
.and_then(|rest| rest.strip_suffix(quote))
227+
{
228+
current = inner;
229+
}
230+
}
231+
current
232+
}
233+
197234
/// 応答全体が属性なしの同名タグ 1 組で包まれていれば、その名前と中身を返す
198235
///
199236
/// プロンプトの `<commit>` 指示に従わず、コミット種別をタグ名にして

‎src/ai/service.rs‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3703,6 +3703,48 @@ mod tests {
37033703
);
37043704
}
37053705

3706+
#[test]
3707+
fn test_process_provider_output_empty_response_does_not_leak_prompt_echo() {
3708+
// Codex は "Reading prompt from stdin..." に続けてプロンプト全文
3709+
// (= staged diff)を stderr へエコーする。空応答エラーに生の stderr を
3710+
// 貼ると、その diff が端末と生成ログの `error` に流れ込む。`error` は
3711+
// 詳細度(`content = "metadata"`)の振り分け対象外なので、
3712+
// 「metadata では diff を残さない」という設計がこの経路だけで破れる。
3713+
let status = exit_status(true);
3714+
let stderr = concat!(
3715+
"Reading prompt from stdin...\n",
3716+
"<changes>\n",
3717+
"diff --git a/src/error.rs b/src/error.rs\n",
3718+
"+const API_TOKEN: &str = \"super-secret-value\";\n",
3719+
"+ let read_error = std::io::Error::last_os_error();\n",
3720+
"</changes>\n",
3721+
"stream error: unexpected EOF\n"
3722+
);
3723+
let result = AiService::process_provider_output(&AiProvider::Codex, status, "", stderr);
3724+
let err = result.unwrap_err().to_string();
3725+
3726+
assert!(
3727+
err.contains("empty response"),
3728+
"空stdoutでは 'empty response' エラーになるべき: {}",
3729+
err
3730+
);
3731+
assert!(
3732+
!err.contains("diff --git"),
3733+
"プロンプトのエコー(diff)がエラー文字列へ漏れている: {}",
3734+
err
3735+
);
3736+
assert!(
3737+
!err.contains("super-secret-value"),
3738+
"diff の中身がエラー文字列へ漏れている: {}",
3739+
err
3740+
);
3741+
assert!(
3742+
err.contains("unexpected EOF"),
3743+
"本当の失敗理由は残すべき: {}",
3744+
err
3745+
);
3746+
}
3747+
37063748
#[test]
37073749
fn test_process_provider_output_codex_stderr_error_keyword_skipped() {
37083750
// Codex: exit code 0 + stdout あり + stderr に "error:" → stderrは無視されて正常
@@ -4793,19 +4835,63 @@ mod tests {
47934835
#[test]
47944836
fn test_clean_message_outer_single_inner_double_quotes() {
47954837
// 外側 single 内側 double のクォートは外側だけが除去される。
4796-
// trim_matches('"') で `'` の両端は変化せず、続く trim_matches('\'') で
4797-
// 外側 `'` が除去された結果、内側の `"text"` がそのまま残ることを保証する。
4838+
// 引用符の種類ごとに順に処理するため、まず `"` では両端が揃わず変化せず、
4839+
// 続く `'` の処理で外側だけが外れて内側の `"text"` がそのまま残る。
47984840
let message = "'\"feat: add feature\"'";
47994841
assert_eq!(AiService::clean_message(message), "\"feat: add feature\"");
48004842
}
48014843

48024844
#[test]
48034845
fn test_clean_message_double_outer_quotes_removed_once() {
4804-
// 連続する複数の同種クォートは trim_matches によって一括で除去される。
4846+
// 連続する複数の同種クォートは、対になっている限り繰り返し除去される。
48054847
let message = "\"\"feat: scope\"\"";
48064848
assert_eq!(AiService::clean_message(message), "feat: scope");
48074849
}
48084850

4851+
#[test]
4852+
fn test_clean_message_keeps_unpaired_quotes() {
4853+
// 片側にしかない引用符は本文の一部であって囲みではない。以前は
4854+
// `trim_matches` が両端を独立に削っていたため、末尾の閉じ引用符だけが
4855+
// 落ちて「引用符が閉じていない件名」がそのままコミットされていた。
4856+
// 下流の欠陥検出(打ち切り / 複数メッセージ / タグ残り)はどれも
4857+
// これを捕まえないため、ここで壊さないことが唯一の防波堤になる。
4858+
4859+
// revert の慣用形式(git revert の既定メッセージと同じ形)
4860+
assert_eq!(
4861+
AiService::clean_message("revert: \"feat: 認証追加\""),
4862+
"revert: \"feat: 認証追加\""
4863+
);
4864+
// 先頭だけが引用符のケースも同じ理由で保持する
4865+
assert_eq!(
4866+
AiService::clean_message("\"foo\" のバグを修正"),
4867+
"\"foo\" のバグを修正"
4868+
);
4869+
// 引用が本文中に複数あっても、囲みでない限り触らない
4870+
assert_eq!(
4871+
AiService::clean_message("chore: rename \"foo\" to \"bar\""),
4872+
"chore: rename \"foo\" to \"bar\""
4873+
);
4874+
// シングルクォートでも同様
4875+
assert_eq!(
4876+
AiService::clean_message("fix: 'foo' の不具合"),
4877+
"fix: 'foo' の不具合"
4878+
);
4879+
}
4880+
4881+
#[test]
4882+
fn test_clean_message_treats_quote_only_response_as_empty() {
4883+
// 対になっていない引用符を保持するようにした副作用で、`"` だけの応答が
4884+
// 「1 文字の有効なメッセージ」として通る余地ができた(以前は両端を独立に
4885+
// 削るので空になり、空応答として次のプロバイダーへ落ちていた)。
4886+
// 中身が無い点は空応答と同じなので、その扱いに戻す。
4887+
assert_eq!(AiService::clean_message("\""), "");
4888+
assert_eq!(AiService::clean_message("\"\"\""), "");
4889+
assert_eq!(AiService::clean_message("'"), "");
4890+
assert_eq!(AiService::clean_message(" \" ' "), "");
4891+
// 中身があるものは当然そのまま
4892+
assert_eq!(AiService::clean_message("\"a\""), "a");
4893+
}
4894+
48094895
// ============================================================
48104896
// extract_error: 追加エッジケース
48114897
// ============================================================

0 commit comments

Comments
 (0)