[#768] fix-parallel-review-errors

[nrslib]

Summary

背景

PR #767 は Issue #758 の Codex SDK Reconnecting... 系エラー retry を修正するものだが、調査中に別の workflow aggregation 問題が見つかった。

対象ログ例:

/Users/nrs/work/git/takt-worktrees/20260527T0119-fix-command-gates/.takt/runs/20260527-050529-implement-using-only-the-files-d1188v/logs/20260527-140530-yo4y0l.jsonl

該当 run では peer-review の reviewers 並列ステップで、7 reviewer 中 6 reviewer は最終的に approve していた。

testing-review              [TESTING-REVIEW:1]
security-review             [SECURITY-REVIEW:1]
coding-review               [CODING-REVIEW:1]
arch-review                 [ARCH-REVIEW:1]
requirements-review         [REQUIREMENTS-REVIEW:1]
qa-review                   [QA-REVIEW:1]

一方で ai-antipattern-review-2nd だけが Phase 1 で provider error になっていた。

ai-antipattern-review-2nd phase 1 execute error
Reconnecting... 2/5 (timeout waiting for child process to exit)

その後、親の reviewers ステップは aggregate rule にマッチできず、次のエラーで peer-review 全体が ABORT した。

Step execution failed: Status not found for step "reviewers": no rule matched after all detection phases
Workflow aborted by step transition

問題

builtins/*/workflows/peer-review.yaml の reviewers は現在、以下の2ルールだけで親ステップを判定している。

rules:
  - condition: all("approved")
    next: COMPLETE
  - condition: any("needs_fix")
    next: fix

そのため、並列 sub-step の一部が error / blocked / その他の非判定状態で終わると、all("approved") も any("needs_fix") も false になり、実際の原因が provider/sub-step error であるにもかかわらず、最終診断が Status not found for step "reviewers" になってしまう。

これはユーザーにとって原因が分かりにくく、retry すべき外部要因なのか、review finding なのか、workflow 定義不備なのかを切り分けにくい。

関連 Issue との差分

• Codex SDK の Reconnecting 系エラーを transient provider failure として扱う #758 は Codex SDK の Reconnecting... を retryable transient failure として扱う問題。直接原因の provider error を減らす対応であり、この Issue の aggregation 診断問題とは別。
• bug: Phase 3 status judgment の throw が既存フォールバックパスをバイパスする #444 は Phase 3 status judgment の throw が fallback をバイパスする問題。今回のケースは Phase 3 以前に sub-step が error になり、親 aggregate rule が非判定状態を扱えない問題。
• Rate limit fallback 機能の追加（参考実装案） #716 は rate limit fallback の参考実装案。今回の対象は rate limit に限らない parallel sub-step error の扱い。

期待する挙動

• 並列 sub-step の一部が error / blocked / rate_limited などで終了した場合、親 parallel step が Status not found ではなく、原因を明示した結果を返す。
• 少なくとも次の情報が診断に含まれる。
  • 失敗した sub-step 名
  • sub-step の status / failureCategory / error
  • aggregate rule が未成立になった理由
• all("approved") / any("needs_fix") のどちらにも該当しない状態を、workflow 定義の曖昧な未マッチとして扱わない。

実装方針案

候補は複数あり得る。

1. ParallelRunner が sub-step error を検出した時点で、親 step response を error / rate_limited / blocked として短絡する。
2. aggregate evaluator に sub-step error 用の明示的な扱いを追加し、親 step の診断を改善する。
3. workflow YAML に any("error") のような条件を足すのではなく、エンジン側で non-rule terminal state を first-class に扱う。

現状の AggregateEvaluator は sub-step の matchedRuleIndex と rule condition だけを見るため、matchedRuleIndex がない error は aggregate 条件から事実上無視される。この構造を見直す必要がある。

受け入れ条件

• parallel sub-step の一部が error になった場合に、親 step が Status not found for step "reviewers" で abort しない。
• 失敗した sub-step 名と error 内容がユーザー向けログ / session log に明示される。
• all("approved") / any("needs_fix") の正常系・fix loop 系の既存挙動は維持される。
• ParallelRunner または aggregate evaluation の unit/integration test に、1 sub-step error + 他 sub-step approved のケースを追加する。
• rate limit / blocked など既存の専用 status は既存ポリシーを壊さずに扱う。

Execution Report

Workflow takt-default completed successfully.

Closes #768

Summary by CodeRabbit

リリースノート

• バグ修正

  • 並列ステップの失敗時に機密情報（APIキー等）が適切にマスクされるよう改善
  • 並列サブステップのエラー情報が親ステップに正しく反映されるよう修正
  • レート制限エラーの処理と情報伝播を改善

• テスト

  • 並列ステップの端末ステータス集約動作を検証する包括的なテスト追加
  • 並列失敗パターンのテストカバレッジ拡張

• リファクタ

  • テストコード内の型アサーション削除による可読性向上

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e6e38e7f-096e-4c0b-8387-45ed8cdd9138

📥 Commits

Reviewing files that changed from the base of the PR and between 16e9ff6 and 855ba47.

📒 Files selected for processing (7)

• src/__tests__/engine-parallel-failure.test.ts
• src/__tests__/engine-rate-limit-fallback.test.ts
• src/__tests__/engine-rate-limit-report-session.test.ts
• src/__tests__/engine-workflow-call.test.ts
• src/__tests__/parallel-runner-terminal-status.test.ts
• src/__tests__/report-phase-retry.test.ts
• src/core/workflow/engine/ParallelRunner.ts

---

📝 Walkthrough

Walkthrough

ParallelRunner が並列サブステップの終端ステータス（error/blocked/rate_limited）を親ステップ結果に集約する実装を追加し、秘匿情報をマスキングして診断精度を向上させた。新規テストスイートで各ケースを検証し、既存テストの期待値を更新した。

Changes

並列ステップ終端ステータスの明示的集約と秘匿情報マスキング

Layer / File(s)	Summary
Terminal ステータス集約ヘルパー実装 src/core/workflow/engine/ParallelRunner.ts	collectTerminalResults、createTerminalParentResult、buildTerminalDiagnostic、firstFailureCategory、firstRateLimitMetadata ヘルパー関数を追加し、error/blocked/rate_limited 終端ステータスを優先度順に親 StepRunResult へ集約。sanitizeSensitiveText によるエラーメッセージの秘匿情報マスキングを導入し、API キーや Authorization ヘッダーが [REDACTED] に置換される。
Terminal ステータス検証テスト src/__tests__/parallel-runner-terminal-status.test.ts	新規テストスイートで、並列サブステップが error/rejected/blocked/rate_limited で終了する6ケースを検証。エラー本文・失敗カテゴリ・集約情報の引き継ぎ、秘匿情報マスキング、プロバイダメタデータの伝播を包括的にアサート。
既存テスト期待値調整 src/__tests__/engine-parallel-failure.test.ts, src/__tests__/engine-rate-limit-fallback.test.ts	並列サブステップ部分失敗・全失敗・レート制限ケースの期待値を更新。abort 理由にサブステップ名とエラーコード含有を確認し、「Status not found」系文言を含まないことを検証。ブロック昇格時の previousResponseSourcePath マッチと秘匿情報マスキングを追加。
型安全性改善（as never 削除） src/__tests__/engine-rate-limit-report-session.test.ts, src/__tests__/engine-workflow-call.test.ts, src/__tests__/report-phase-retry.test.ts	rate_limited ステータスの型キャスト（as never）を削除し、シンプルなリテラル指定 status: 'rate_limited' に統一。複数テストファイルで一貫性を確保。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• nrslib/takt#763: ParallelRunner の並列サブステップ失敗処理とクオリティゲート実行が同じ終端ステータス集約パスに関連し、並列失敗時の診断フロー全体に影響する共通実装領域。

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	タイトル「[#768] fix-parallel-review-errors」は、PR目的であるissue #768の並列レビューのエラー処理の修正を明確に表現しており、変更の主目的を簡潔かつ具体的に示している。
Linked Issues check	✅ Passed	変更内容がissue #768の全要件を満たしている。ParallelRunnerがterminal statusの処理を共通化し、diagnosticsが含まれ、normal/fix-loop動作が維持され、テストが追加されている。
Out of Scope Changes check	✅ Passed	全ての変更がissue #768のスコープ内である。ParallelRunnerの端末ステータス処理強化、関連テスト更新、およびリファクタリング（statusの型アサーション削除など）は全てスコープ内。
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch takt/768/fix-parallel-review-errors

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}