diff --git a/BACKLOG.md b/BACKLOG.md index 9d210e28..e9043132 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -107,7 +107,8 @@ CC-001/CC-002 were consumed by PR #24 fix bundle inline, with no standalone entr | CC-568 | 🟢 someday | `/mem-distill` Case→Strategy 機械式提升:對 `episodes.jsonl` 既有結構化欄位做 count/cluster 門檻判定,取代逐次主觀「感覺像 pattern」的判斷;依賴 [[CC-567]] 的 outcome 證據決定是否值得做(2026-08-25 memory 架構設計討論) | memory/DX | 2026-08-25 | — | P2 | retrieval | | CC-569 | 🟢 someday | `pmctl task` / `context pack` 擴充 working-memory 敘事欄位(`selected_memories`/`rejected_paths`/`blockers`/`next_action`):延伸既有 schema,不新建第二個「現在在幹嘛」真相來源;依賴 [[CC-567]] 證明有價值後再排(2026-08-25 memory 架構設計討論) | memory/DX | 2026-08-25 | — | P2 | design | | CC-570 | 🟢 someday | Fact/Case/Strategy `memory_function`/`memory_subtype` metadata 分類法:先蒐集 [[CC-567]] 的 applied/outcome 證據,再決定值不值得建分類機制——不憑直覺先建立稅務式標籤(2026-08-25 memory 架構設計討論;外部文章優先序建議相反,本 repo 刻意反過來) | memory/DX | 2026-08-25 | — | P3 | retrieval | -| CC-571 | 🔵 active | `_ctx_fts_rebuild`/`_ctx_index_file` 共用的 sqlite atomic-script 缺口:DROP+CREATE+INSERT 未加 `-bail`(實測 sqlite3 CLI 預設不會在錯誤時中止,單靠 BEGIN/COMMIT 不足)、呼叫端不檢查回傳值、`_ctx_index_file` 還有第三個獨立 bug(`rm -f` 蓋掉 sqlite3 真實 exit code);`/simplify` altitude review 抓到手足函式同缺陷,範圍已擴大涵蓋兩者([[CC-548]] spike 的 Open risks 側面發現,非本票 tokenizer 範圍) | memory/ops | 2026-08-26 | — | P2 | hygiene | +| CC-571 | ✅ done | `_ctx_fts_rebuild`/`_ctx_index_file` 共用的 sqlite atomic-script 缺口:DROP+CREATE+INSERT 未加 `-bail`(實測 sqlite3 CLI 預設不會在錯誤時中止,單靠 BEGIN/COMMIT 不足)、呼叫端不檢查回傳值、`_ctx_index_file` 還有第三個獨立 bug(`rm -f` 蓋掉 sqlite3 真實 exit code);`/simplify` altitude review 抓到手足函式同缺陷,範圍已擴大涵蓋兩者([[CC-548]] spike 的 Open risks 側面發現,非本票 tokenizer 範圍) | memory/ops | 2026-08-26 | pr:#539 | P2 | hygiene | +| CC-572 | ✅ done | pr-gate synthesis retry(sequential/parallel 兩條路徑)留下已存在但 0 bytes 的 `$OUTPUT_FILE`,executor 的 patch 工具仍可能選擇 Update File 而非 Add File 語意,對空內容找不到 context line 而崩潰(`apply_patch verification failed`);CC-571 gate saga 連續四輪協定失敗實測發現(2026-08-26) | gate/ops | 2026-08-26 | pr:#541 | P2 | hygiene | --- @@ -3335,7 +3336,7 @@ machinery,是憑一篇文章的直覺蓋機制,屬於本 repo 已經吃過 --- -## CC-571 — sqlite atomic-script 缺口:`_ctx_fts_rebuild`/`_ctx_index_file` 🔵 active +## CC-571 — sqlite atomic-script 缺口:`_ctx_fts_rebuild`/`_ctx_index_file` **Problem**: `runtime/lib/pmctl-context.sh` 的 `_ctx_fts_rebuild()` 對 `content_fts` 做 `DROP TABLE` → `CREATE VIRTUAL TABLE` → 兩個 `INSERT ... SELECT`, @@ -3393,4 +3394,57 @@ best-effort 加速層,宣稱「re-indexed」等於說謊)。新增對應 reg `runtime/lib/memory.sh` 的 `memory_usage_commit`(既有的 `-bail` atomic-script 先例,本票的 helper 命名與理由都直接引用它,而非各自重新推導)。 +**Update 2026-08-26(done,pr:#539)**:pr-gate 5 輪後 GO(critic/qa-tester/ +architecture-reviewer/security-reviewer 全數 approve)。前兩輪是真實發現並已修正: +round 1 critic-F001——stderr 有印降級訊息,但 stdout 的成功摘要行本身仍是無條件 +「N indexed, M skipped」,對只看 stdout/exit code 的呼叫端是矛盾摘要,改成把降級 +狀態直接併入 stdout 摘要行本身;round 2 critic-F001——首次建置索引失敗時(rebuild +前 `content_fts` 根本不存在),訊息卻說「現有索引維持」,改為依 rebuild 前是否已有 +`content_fts` 分支措辭。中間另有 3 輪是 gate 執行環境本身的 synthesis 協定不穩定 +(`apply_patch` 在同一份 result 檔案上多次操作互相衝突、`findings_union`/ +`disagreement` 結構不一致),與程式碼無關,重跑收斂。`tests/bin/run-all-tests.sh` +104 passed 0 failed。狀態旗標本次於 main 更新後立即補記——同一 session 已因此類 +漏更新撞過三次(CC-567/CC-533/CC-015),這次差點又漏,補上教訓:**合併前**就該 +在 PR 裡帶上狀態翻轉,合併後才想起來永遠比合併前想起來更容易忘記。 + +--- + +## CC-572 — pr-gate synthesis 重試留下空但存在的 result 檔案,patch 工具語意混淆 + +**Problem**: CC-571 的 pr-gate saga 連續遇到 4 輪協定失敗,其中兩類錯誤反覆出現: +`apply_patch verification failed: invalid patch: multiple operations target ` +與 `Failed to find expected lines in : ...`。追查後發現:sequential 模式的 +synthesis 重試(`runtime/bin/pr-gate.sh` 約 line 2748)在重試前用 `: > "$OUTPUT_FILE"` +把結果檔案**清空但保留路徑存在**;parallel 模式的 synthesis 重試(約 line 3600 附近的 +迴圈)則完全沒有清空或移除,重試時 `$OUTPUT_FILE` 仍是第一次嘗試的完整內容。兩者都 +讓 executor 的 patch 工具面對一個「路徑存在」的檔案,可能因此選擇 `Update File` +(需要定位既有內容做編輯)而非 `Add File`(單純新建)語意——對 0 bytes 或即將整份 +重寫的檔案,`Update File` 語意本質上找不到可定位的 context line,因而崩潰。 + +**Why**: reviewer-protocol 的重試路徑(同檔案內,寫到全新的 +`reviewer---retry1.md` 路徑)從未出現過這個問題——因為那個路徑保證是全新 +的,patch 工具沒有選錯語意的空間。Synthesis 的兩條重試路徑都固定用同一個 +`$OUTPUT_FILE`(這個路徑本身是使用者看得到的 canonical gate 結果路徑,不能像 +reviewer 重試一樣改路徑),只能改成每次重試前把該路徑**整個移除**(而非清空), +逼 patch 工具只能選擇 `Add File`。 + +**Requirement**: +1. 兩條 synthesis 重試路徑(sequential/parallel)在重新 dispatch 前,都必須讓 + `$OUTPUT_FILE` 這個路徑真正不存在(而非僅清空內容),逼 patch 工具走 + `Add File` 而非 `Update File`。 +2. Regression fixtures 驗證重試發生時 `$OUTPUT_FILE` 在第二次 dispatch **開始前** + 確實不存在,且既有 synthesis-protocol 測試全數維持綠燈。 + +**Non-goals**: 不改變 synthesis 重試次數(維持 1 次,不重新開放 CC-544 已被否決的 +「重試把失敗變成通過」爭議——本票的重試機制本來就誠實回報協定失敗,不受影響); +不修改 reviewer-protocol 既有的重試機制(已經是正確模式,不需要改);不嘗試修正 +codex 自己的 apply_patch 工具實作(不在本 repo 控制範圍)。 + +**Cross-link**: [[CC-571]](gate saga 實測發現本問題的來源)。 + +**Update 2026-08-26(done,pr:#541)**:兩條路徑都已修好,新增迴歸測試直接斷言 +重試發生時該路徑真的不存在(而非僅清空)。pr-gate 首輪 GO(未在該次 gate run +自身觸發 synthesis retry,修復是靠直接比對過往失敗 log 的根因+白箱迴歸測試 +驗證,非現場實戰)。`tests/bin/run-all-tests.sh` 104 passed 0 failed。 + --- diff --git a/runtime/bin/pr-gate.sh b/runtime/bin/pr-gate.sh index e383de7b..f06d47a4 100755 --- a/runtime/bin/pr-gate.sh +++ b/runtime/bin/pr-gate.sh @@ -804,6 +804,18 @@ gate_dispatch_command() { fi } +# Removing the path (not truncating it) matters -- observed in production, +# an executor's patch tool can choose an "Update File" operation (which +# locates existing content to edit) against a path that still EXISTS on +# disk, even truncated to 0 bytes -- and that operation then fails +# ("Failed to find expected lines", "invalid patch: multiple operations +# target ") because there is no content to locate. Only removing the +# path forces an unambiguous "Add File", matching the reviewer-retry path's +# already-reliable brand-new-filename behavior. +gate_clear_retry_target() { + rm -f "$1" +} + # Every supported executor now dispatches an INDEPENDENT subprocess (codex `codex # exec`, claude headless `claude --print`) and writes the result in-process, which # the gate then integrity-checks. This flag is the seam where a future @@ -2742,10 +2754,11 @@ BRIEF_EOF for _seq_attempt in 1 2; do if [[ "$_seq_attempt" -eq 2 ]]; then # The brief has the executor CREATE the file on the first reviewer and - # APPEND for the rest, so a retry must start from an empty document: + # APPEND for the rest, so a retry must start from a clean slate: # appending to a rejected, half-ordered one compounds the defect the - # retry exists to fix. - : > "$OUTPUT_FILE" + # retry exists to fix. See gate_clear_retry_target for why this removes + # the path rather than truncating it. + gate_clear_retry_target "$OUTPUT_FILE" # Same trust boundary as the parallel retry: the reason quotes ids read # from the rejected artifact, so flatten newlines and bound the length # before it becomes a YAML block scalar in a privileged brief. @@ -2753,7 +2766,7 @@ BRIEF_EOF _seq_reason_line="${_seq_reason_line//$'\r'/ }" [[ "${#_seq_reason_line}" -le 800 ]] \ || _seq_reason_line="${_seq_reason_line:0:800}~" - printf '\ncorrection_retry: |\n The first attempt was REJECTED for exactly this reason:\n\n %s\n\n %s has been emptied. Rebuild it completely, in the required section\n order, from the same reviewer evidence -- fix that specific defect and do\n not change any other section to compensate.\n' \ + printf '\ncorrection_retry: |\n The first attempt was REJECTED for exactly this reason:\n\n %s\n\n %s has been removed. Rebuild it completely from scratch, in the\n required section order, from the same reviewer evidence -- fix that\n specific defect and do not change any other section to compensate.\n' \ "$_seq_reason_line" "$OUTPUT_FILE" >> "$BRIEF_FILE" say ' [sequential] retrying once after %s.\n' "$_seq_reason" fi @@ -3619,6 +3632,15 @@ SBRIEF_P2 || _synthesis_reason_line="${_synthesis_reason_line:0:800}~" printf '\ncorrection_retry: |\n The first synthesis attempt was REJECTED for exactly this reason:\n\n %s\n\n Fix that specific defect. Copied coverage/inventory/test-gap fields are\n restored by the shell from the embedded reviewer_result_v1 documents; do not\n retype them. Rebuild grouping, disagreement, confirmation, and seed fields\n from those same documents -- do not change any other section to compensate.\n' \ "$_synthesis_reason_line" >> "$SYNTHESIS_BRIEF" + # Unlike the reviewer-protocol retry above (which writes to a + # brand-new path, e.g. reviewer---retry1.md), the synthesis + # retry re-dispatches to the SAME fixed $OUTPUT_FILE path. See + # gate_clear_retry_target for why this removes the path rather than + # leaving attempt 1's content in place. The brief already + # tells the model to rebuild every field fresh from the embedded + # reviewer context, so nothing here depends on attempt 1's on-disk + # content surviving into attempt 2. + gate_clear_retry_target "$OUTPUT_FILE" fi say ' [synthesis attempt %d] running PM consolidation...\n' "$_synthesis_attempt" SYNTHESIS_DISPATCH_CMD="$(gate_dispatch_command "$EXECUTOR" "$SYNTHESIS_BRIEF" "$WORK_DIR" "$DISPATCH_MODEL" "$DISPATCH_SANDBOX" "$DISPATCH_APPROVAL" "$TIMEOUT" "$DISPATCH_ISOLATION" "$DISPATCH_EFFORT")" || exit 2 diff --git a/tests/shell/test-pr-gate.sh b/tests/shell/test-pr-gate.sh index 5096e7ef..920989cc 100755 --- a/tests/shell/test-pr-gate.sh +++ b/tests/shell/test-pr-gate.sh @@ -120,6 +120,27 @@ done reviewer_name="$(awk '$1 == "Reviewer:" { print $2; exit }' "$brief_file")" : "${reviewer_name:=stub-reviewer}" +# Record whether the synthesis/sequential result path exists on disk at the +# very start of THIS dispatch (before this stub or anything else in this +# invocation touches it) -- the property a synthesis retry must uphold is +# that the path is gone (not just empty) before the retry's own write +# begins. Must run before any other block below that might create the file. +if [[ -n "${CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR:-}" ]] \ + && { [[ "$brief_file" == *-synthesis.md ]] || grep -q '^goal: Sequential ' "$brief_file"; }; then + _output_exists_path=$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}') + mkdir -p "$CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR" + # Each gate run's synthesis dispatches at most twice (initial + one + # retry), so a fixed first/second pair of files is simpler than a + # counted/globbed sequence for a caller that only ever expects two checks. + _output_exists_state="exists" + [[ -n "$_output_exists_path" && -e "$_output_exists_path" ]] || _output_exists_state="absent" + if [[ ! -e "$CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR/first" ]]; then + printf '%s\n' "$_output_exists_state" > "$CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR/first" + else + printf '%s\n' "$_output_exists_state" > "$CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR/second" + fi +fi + # CC-541: capture whatever QA_RULES_DIR value (if any) this dispatch inherited, # so tests can assert pr-gate.sh's host-side resolution reached the reviewer # subprocess env without needing a real codex model to interpret it. @@ -11887,6 +11908,65 @@ test_parallel_synthesis_retry_brief_bounds_long_reason() { pass "$name" } +# Behavior: a synthesis retry (either mode) must remove $OUTPUT_FILE before +# re-dispatching, not just leave the first attempt's content sitting on +# disk. Observed in production: an executor's patch tool can choose an +# "Update File" operation against a path that still exists (even truncated +# to empty), and that operation then fails outright because there is no +# matching content to locate -- a hard tool failure unrelated to the +# actual synthesis content. Only full removal forces an unambiguous +# "Add File" the same way a brand-new path would. +# Steps: force a first-attempt synthesis failure (existing malformed-seed +# mutation); the capture hook in the stub records, at the very start of +# EACH synthesis dispatch (before the stub or anything else writes to the +# path), whether $OUTPUT_FILE existed at that instant. Assert the first +# check says "exists" (the reviewer-append step already wrote content before +# synthesis attempt 1 runs) and the second (retry) check says "absent". +# Shared by both modes because the property under test -- and the fixture +# setup to exercise it -- is identical; only --mode differs. Sequential and +# parallel dispatch through separately-authored retry loops in +# runtime/bin/pr-gate.sh (the sequential one used to truncate the file +# rather than remove it), so both are exercised as their own named result. +_test_synthesis_retry_removes_output_file_before_redispatch() { + local mode="$1" test_prefix="$2" + local name="${test_prefix}/retry-removes-output-file-before-redispatch" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code=0 + local checks="$TMP_ROOT/$name/checks" + mkdir -p "$dir" "$checks" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_SYNTHESIS_PROTOCOL_MUTATION=malformed-seed \ + CODEX_GATE_STUB_SYNTHESIS_PROTOCOL_MUTATION_ONLY_FIRST=1 \ + CODEX_GATE_CAPTURE_OUTPUT_EXISTS_DIR="$checks" \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode "$mode" + code=$? + set -e + [[ "$code" -eq 0 ]] || { fail "$name" "did not recover: code=$code $(tail -n 5 "$err" 2>/dev/null)"; return; } + [[ -f "$checks/second" ]] || { + fail "$name" "second dispatch's existence check was not captured: $(find "$checks" -maxdepth 1 -printf '%P ' 2>/dev/null)" + return + } + assert_file_contains "$name" "$checks/first" "exists" || return + assert_file_contains "$name" "$checks/second" "absent" || return + pass "$name" +} + +# Behavior: parallel-mode wrapper for the shared check above. +test_parallel_synthesis_retry_removes_output_file_before_redispatch() { + _test_synthesis_retry_removes_output_file_before_redispatch parallel synthesis-protocol +} + +# Behavior: sequential-mode wrapper for the shared check above. +test_sequential_synthesis_retry_removes_output_file_before_redispatch() { + _test_synthesis_retry_removes_output_file_before_redispatch sequential sequential-protocol +} + # Behavior: a stale subject binding is NOT retried in sequential mode. # Steps: bind the synthesis to a different scope digest, then assert the gate # refuses the retry and says so. @@ -12985,6 +13065,8 @@ run_test test_synthesis_protocol_diagnostics_name_the_defect run_test test_synthesis_protocol_diagnostics_neutralize_injected_ids run_test test_parallel_synthesis_retry_brief_carries_reason run_test test_parallel_synthesis_retry_brief_bounds_long_reason +run_test test_parallel_synthesis_retry_removes_output_file_before_redispatch +run_test test_sequential_synthesis_retry_removes_output_file_before_redispatch run_test test_sequential_protocol_recovers_on_retry run_test test_sequential_protocol_refuses_stale_subject_retry run_test test_sequential_retry_brief_bounds_long_reason