diff --git a/BACKLOG.md b/BACKLOG.md index db3c04f8..9d210e28 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -107,6 +107,7 @@ 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 | --- @@ -3333,3 +3334,63 @@ machinery,是憑一篇文章的直覺蓋機制,屬於本 repo 已經吃過 **Dependencies**: 前置 = [[CC-567]] shipped + 觀察窗證據。P3,不預設排入 milestone。 --- + +## CC-571 — sqlite atomic-script 缺口:`_ctx_fts_rebuild`/`_ctx_index_file` 🔵 active + +**Problem**: `runtime/lib/pmctl-context.sh` 的 `_ctx_fts_rebuild()` 對 +`content_fts` 做 `DROP TABLE` → `CREATE VIRTUAL TABLE` → 兩個 `INSERT ... SELECT`, +整段用 heredoc 餵給 `sqlite3 "$db" >/dev/null`,沒有 `BEGIN`/`COMMIT`。兩個呼叫端 +(`pmctl_context_index` 約 line 795、`pmctl_context_update` 約 line 1126)都是裸呼叫 +`_ctx_fts_rebuild "$db"`,不檢查回傳值,之後照樣印「context index/update」成功訊息。 + +**Why**: 直接實測證實這不是理論風險。用一個蓄意中途出錯的重建腳本測試: +1. 不加 `-bail`:sqlite3 CLI 預設遇到錯誤只印訊息、**不中止**,照樣跑到 `COMMIT` + (若有包 transaction 也一樣會提交半成功的內容);exit code 雖然是 1,但呼叫端 + 從不檢查。 +2. 加 `-bail` 後才會在第一個錯誤處真正中止,交易維持未提交,行程結束時連線關閉 + 觸發自動 rollback,舊的 `content_fts` 完整保留(已用最小 repro 驗證)。 + +三個問題疊在一起:(a) 沒有 atomicity——失敗可能留下半建或整個消失的表;(b) 沒有 +`-bail`,單靠 `BEGIN`/`COMMIT` 不足以達成 (a) 的保護;(c) 呼叫端不檢查回傳值, +即使 (a)(b) 都修好,使用者也不會知道索引其實是舊的(rollback 後)卻顯示重建成功。 +本票是 [[CC-548]] spike 過程中在 Open risks 側面發現的既有缺口,與該票的 tokenizer +判斷(AMBER,暫緩)完全無關;使用者已明確要求只處理這個 bug,不連動 trigram 切換。 + +**Requirement**: +1. `_ctx_fts_rebuild` 的 DROP/CREATE/INSERT 序列包進單一交易(`BEGIN + IMMEDIATE`…`COMMIT`),並對 `sqlite3` 呼叫加 `-bail`(或等效機制),確保任何一步 + 出錯都會在該步中止、交易不提交,使既有 `content_fts` 保持完整可查詢,而不是 + 半建或消失。 +2. `_ctx_fts_rebuild` 的失敗必須讓呼叫端可辨——回傳非零,且兩個呼叫端 + (`pmctl_context_index`/`pmctl_context_update`)改為檢查其回傳值:失敗時不得 + 印「成功」字樣的訊息,改為誠實回報「FTS 索引重建失敗,仍使用既有索引」一類的 + 降級狀態(比照本 repo既有 `usage_store: error`/`resolution_issues` 誠實回報慣例, + 不阻斷整體 index/update 流程——FTS 只是加速層,非唯一查詢路徑,LIKE fallback + 仍可用)。 +3. Regression fixtures:模擬重建腳本中途失敗(例如注入一個會觸發 SQL 錯誤的條件), + 斷言 (a) 舊 `content_fts` 內容不變、(b) `_ctx_fts_rebuild` 回傳非零、(c) 呼叫端 + 印出的訊息誠實反映失敗、不宣稱成功。 + +**Non-goals**: 不改 FTS5 tokenizer(unicode61 維持不變,[[CC-548]] 已判 AMBER 暫緩); +不新增 schema 欄位或 `index_meta` 版本追蹤;不處理 query-during-rebuild 的 +讀者可見性問題本身(rollback 後舊表持續可查詢,交易保護已隱含解決多數場景)。 + +**Update 2026-08-26(範圍擴大,實作中)**:`/simplify` 的 altitude review 在同一輪 +reuse/簡化確認裡抓到手足函式同缺陷——`_ctx_index_file()`(`pmctl_context_update` +另一個呼叫路徑,寫的是 files/symbols/file_chunks 主索引資料,非 FTS 加速層)用 +`BEGIN;`…`COMMIT;` 但同樣沒加 `-bail`;直接測試還額外找到第三個獨立 bug:其函式 +本體最後一行是 `sqlite3 ...; rm -f "$tmpf"`,函式回傳值變成 `rm` 的 exit code(幾乎 +恆為 0),完全蓋掉 sqlite3 真正的失敗狀態,即使先前已加 `-bail` 也測不出來。範圍 +擴大為:兩個函式共用同一個新抽出的 `_ctx_sqlite_exec_atomic` helper(單一 +`-bail` 呼叫來源,同時解決 reuse review 指出的「兩處各自重新推導同一手法」); +`_ctx_index_file` 明確 `return "$rc"`(在 `rm` 之前先擷取),且其唯一呼叫端 +(`pmctl_context_update`)失敗時視為**致命**(不同於 FTS——這是主索引資料而非 +best-effort 加速層,宣稱「re-indexed」等於說謊)。新增對應 regression fixtures +(`_ctx_index_file` 回傳碼、`pmctl_context_update` 失敗時不宣稱成功)。使用者已 +確認此擴大屬於「同一個 bug」範圍內的自然延伸,非另開新工。 + +**Cross-link**: [[CC-548]](spike 中發現本缺口,Open risks 段落)。也可見 +`runtime/lib/memory.sh` 的 `memory_usage_commit`(既有的 `-bail` atomic-script +先例,本票的 helper 命名與理由都直接引用它,而非各自重新推導)。 + +--- diff --git a/runtime/lib/pmctl-context.sh b/runtime/lib/pmctl-context.sh index 5f96b49d..a2387c92 100755 --- a/runtime/lib/pmctl-context.sh +++ b/runtime/lib/pmctl-context.sh @@ -602,20 +602,53 @@ _ctx_generate_file_sql() { done < <(_ctx_chunk_file "$abs_path" "$lang") } +# CC-571: shared atomic-script executor for this file's two DROP/CREATE/ +# INSERT-style rebuild scripts (_ctx_index_file, _ctx_fts_rebuild). Runs the +# caller-supplied SQL (via stdin, which the caller wraps in its own +# BEGIN IMMEDIATE/COMMIT) through `sqlite3 -bail`. +# +# `-bail` is not optional decoration: confirmed by direct reproduction, the +# sqlite3 CLI's default behavior on a mid-script SQL error is to print the +# error and keep executing subsequent statements (it does NOT stop) -- so a +# bare BEGIN...COMMIT without -bail still reaches and executes COMMIT after +# silently skipping the failed statement, committing a half-built table. +# With -bail, an error aborts the script immediately, the transaction is +# left open, and the sqlite3 process exiting closes the connection, which +# triggers an automatic ROLLBACK -- prior committed state is left fully +# intact. This mirrors the `-bail`-based atomic-script pattern already +# established in memory.sh's memory_usage_commit (~line 404) for the same +# reason; that instance also captures stderr and retries on a lock error, +# which neither caller here currently needs (both run at most once per +# `context index`/`context update` invocation, not under contention). +# +# Returns sqlite3's exit code. Callers MUST check it -- this function does +# not decide whether a failure here is fatal to the caller; that differs +# per call site (see CC-571 Requirement 2). +_ctx_sqlite_exec_atomic() { + local db="$1" + sqlite3 -bail "$db" >/dev/null +} + # ── Single-file index (used by pmctl_context_update) ────────────────────────── _ctx_index_file() { local db="$1" abs_path="$2" rel_path="$3" - local tmpf + local tmpf rc tmpf="$(mktemp /tmp/ctx-XXXXXX.sql)" { printf 'PRAGMA busy_timeout=5000;\n' - printf 'BEGIN;\n' + printf 'BEGIN IMMEDIATE;\n' _ctx_generate_file_sql "$abs_path" "$rel_path" printf 'COMMIT;\n' } > "$tmpf" - sqlite3 "$db" < "$tmpf" >/dev/null + _ctx_sqlite_exec_atomic "$db" < "$tmpf" + rc=$? rm -f "$tmpf" + # rc must be captured before `rm` -- rm's own exit status would otherwise + # become this function's return value regardless of whether sqlite3 + # actually succeeded (confirmed by direct reproduction: the original + # `sqlite3 ... ; rm -f "$tmpf"` shape always returned 0). + return "$rc" } # ── FTS5 index rebuild ───────────────────────────────────────────────────────── @@ -628,8 +661,9 @@ _ctx_fts_rebuild() { # each hit's real bounded span instead of faking line_end=line_start (the # pack/query contract advertises line_start/line_end as the actual chunk or # symbol extent -- CC-505 Req 3 gate finding critic-F001). - sqlite3 "$db" >/dev/null <<'SQLFTS' + _ctx_sqlite_exec_atomic "$db" <<'SQLFTS' PRAGMA busy_timeout=5000; +BEGIN IMMEDIATE; DROP TABLE IF EXISTS content_fts; CREATE VIRTUAL TABLE content_fts USING fts5(ref, text, line_end UNINDEXED); INSERT INTO content_fts(ref, text, line_end) @@ -639,6 +673,7 @@ INSERT INTO content_fts(ref, text, line_end) SELECT f.path || ':' || fc.line_start, TRIM(COALESCE(fc.heading, '') || ' ' || COALESCE(fc.text, '')), fc.line_end FROM file_chunks fc JOIN files f ON fc.file_id = f.id WHERE TRIM(COALESCE(fc.heading, '') || ' ' || COALESCE(fc.text, '')) != ''; +COMMIT; SQLFTS } @@ -790,12 +825,38 @@ _ctx_index_tree() { # Rebuild only for changed files or a path-count change (pure deletions). local _fts_present _fts_present="$(sqlite3 "$db" "SELECT count(*) FROM sqlite_master WHERE type='table' AND name='content_fts';" 2>/dev/null || printf '0')" + # CC-571: FTS rebuild is a best-effort acceleration layer, not the only + # query path (LIKE fallback remains available), so a failed rebuild does + # not fail the overall index -- but neither the stderr diagnostic NOR the + # final stdout summary may read as an unqualified success (gate finding + # critic-F001, round 1: printing the diagnostic on stderr while stdout + # still said a bare "N indexed, M skipped" is a contradictory summary -- + # a caller that only looks at stdout, or at exit code 0, saw only + # success). The previous content_fts (rolled back to, not left + # half-built -- see _ctx_fts_rebuild) is now stale relative to the + # indexed content above. + local _fts_rebuild_note="" if (( indexed > 0 || found != ${#_ctx_db_mtimes[@]} )) || [[ "$_fts_present" != "1" ]] \ || [[ "$_force_reextract" -eq 1 ]]; then - _ctx_fts_rebuild "$db" + if ! _ctx_fts_rebuild "$db"; then + # CC-571 gate finding critic-F001 (round 2, gate-20260826-021038-ac0bc2): + # when content_fts did not exist before this attempt (first-time + # build), a failed rebuild's rollback leaves NO FTS table at all -- + # "existing (now stale) FTS index retained" would be false in that + # case, since there is no existing index to retain. _fts_present was + # captured before the rebuild attempt, so it still reflects the + # pre-rebuild state here. + if [[ "$_fts_present" == "1" ]]; then + printf 'pmctl context index: FTS index rebuild failed; existing (now stale) FTS index retained, LIKE fallback still available\n' >&2 + else + printf 'pmctl context index: FTS index rebuild failed; no FTS index available, LIKE fallback only\n' >&2 + fi + _fts_rebuild_note=' (FTS index degraded: rebuild failed, stale index retained)' + [[ "$_fts_present" == "1" ]] || _fts_rebuild_note=' (FTS index degraded: rebuild failed, no FTS index available)' + fi fi - printf 'context index: %d indexed, %d skipped\n' "$indexed" "$skipped" + printf 'context index: %d indexed, %d skipped%s\n' "$indexed" "$skipped" "$_fts_rebuild_note" printf 'db: %s\n' "$db" } @@ -1122,9 +1183,33 @@ pmctl_context_update() { return 1 fi local rel_path="${real_path#"$canon_root/"}" - _ctx_index_file "$db" "$real_path" "$rel_path" - _ctx_fts_rebuild "$db" - printf 'context update: re-indexed %s\n' "$rel_path" + # CC-571: unlike the FTS rebuild below, a failed _ctx_index_file is + # fatal to this command -- it writes the primary files/symbols/ + # file_chunks data, not a best-effort acceleration layer, so a rollback + # here means the file's content is genuinely not reflected. Reporting + # "re-indexed" anyway would be a false claim, not a degraded state. + if ! _ctx_index_file "$db" "$real_path" "$rel_path"; then + printf 'pmctl context update: failed to index %s; index not updated for this file\n' "$rel_path" >&2 + return 1 + fi + # CC-571: see the matching comment in pmctl_context_index -- neither the + # stderr diagnostic nor the final stdout summary may read as an + # unqualified success when the FTS rebuild fails (gate finding + # critic-F001, round 1), and the message must not claim a stale index + # is "retained" when no content_fts existed before this attempt + # (gate finding critic-F001, round 2). + local _fts_rebuild_note="" _fts_present_before_update + _fts_present_before_update="$(sqlite3 "$db" "SELECT count(*) FROM sqlite_master WHERE type='table' AND name='content_fts';" 2>/dev/null || printf '0')" + if ! _ctx_fts_rebuild "$db"; then + if [[ "$_fts_present_before_update" == "1" ]]; then + printf 'pmctl context update: FTS index rebuild failed; existing (now stale) FTS index retained, LIKE fallback still available\n' >&2 + _fts_rebuild_note=' (FTS index degraded: rebuild failed, stale index retained)' + else + printf 'pmctl context update: FTS index rebuild failed; no FTS index available, LIKE fallback only\n' >&2 + _fts_rebuild_note=' (FTS index degraded: rebuild failed, no FTS index available)' + fi + fi + printf 'context update: re-indexed %s%s\n' "$rel_path" "$_fts_rebuild_note" else # No path given: full incremental scan (same as index with mtime check) pmctl_context_index "$repo_root" diff --git a/tests/shell/test-pmctl-context.sh b/tests/shell/test-pmctl-context.sh index 968f16e4..5362401d 100755 --- a/tests/shell/test-pmctl-context.sh +++ b/tests/shell/test-pmctl-context.sh @@ -4354,6 +4354,268 @@ case_context_fts5_availability_is_cached() { pass "$name" } +# Behavior (CC-571): _ctx_fts_rebuild wraps its DROP+CREATE+INSERT sequence +# in BEGIN IMMEDIATE/COMMIT and invokes sqlite3 with -bail. Without -bail, +# direct reproduction during implementation showed the sqlite3 CLI's default +# behavior on a mid-script SQL error is to print the error and KEEP +# executing subsequent statements -- it does not stop -- so a bare +# BEGIN...COMMIT alone would still reach and execute COMMIT after silently +# skipping the failed statement, committing a half-built content_fts. With +# -bail, an error aborts before COMMIT, leaving the transaction open; the +# sqlite3 process exiting then closes the connection, which triggers an +# automatic ROLLBACK, leaving the previous content_fts fully intact. +# Steps: index a real fixture repo (creates a real content_fts with real +# rows); drop the `files` table `_ctx_fts_rebuild`'s INSERT...JOIN depends on +# to force a genuine SQL failure mid-rebuild; call `_ctx_fts_rebuild` again +# directly; assert it returns non-zero AND the old content_fts row is still +# present and queryable (not half-built, not dropped-and-not-recreated). +case_ctx_fts_rebuild_rollback_preserves_old_index_on_failure() { + local name="pmctl context: _ctx_fts_rebuild rolls back on failure, preserving the previous index" + should_run "$name" || return 0 + + local fix_repo="$tmp_root/fix-repo-fts-rollback" + make_fixture_repo "$fix_repo" + local err="$tmp_root/fts-rollback-setup.err" + "$PMCTL" context index "$fix_repo" > /dev/null 2> "$err" \ + || { fail "$name" "setup: initial index failed: $(<"$err")"; return 0; } + + local db="$fix_repo/.pm-dispatch/ctx/context.db" + local before_rows + before_rows="$(sqlite3 "$db" "SELECT count(*) FROM content_fts;" 2>/dev/null)" + if [[ -z "$before_rows" || "$before_rows" -eq 0 ]]; then + fail "$name" "setup: expected a populated content_fts after initial index, got $before_rows rows" + return 0 + fi + + # Force a genuine mid-rebuild SQL failure: the rebuild's own INSERT...JOIN + # against `files` can no longer resolve once that table is gone. + sqlite3 "$db" "DROP TABLE files;" 2>/dev/null \ + || { fail "$name" "setup: could not drop files table"; return 0; } + + local out + out="$tmp_root/fts-rollback.out" + bash -c ' + set -uo pipefail + # shellcheck source=runtime/lib/pmctl-context.sh + . "$1/lib/pmctl-context.sh" + _ctx_fts_rebuild "$2" + echo "rc=$?" + ' bash "$REPO_ROOT/runtime" "$(ctx_fixture_target "$db")" > "$out" 2>>"$err" || true + + local rc + rc="$(grep '^rc=' "$out" | cut -d= -f2)" + if [[ "$rc" -eq 0 ]]; then + fail "$name" "_ctx_fts_rebuild reported success (rc=0) despite the forced SQL failure" + return 0 + fi + + local after_rows + after_rows="$(sqlite3 "$db" "SELECT count(*) FROM content_fts;" 2>/dev/null)" + if [[ "$after_rows" != "$before_rows" ]]; then + fail "$name" "content_fts row count changed after a failed rebuild: before=$before_rows after=$after_rows (expected unchanged -- rollback should have preserved it)" + return 0 + fi + pass "$name" +} + +# Behavior (CC-571): _ctx_index_file's own return code must reflect whether +# its sqlite3 write actually succeeded, not the unconditional `rm -f` cleanup +# that used to be the function's last statement (confirmed by direct +# reproduction during implementation: `sqlite3 ; rm -f "$tmpf"` as a +# function body always returns 0, the exit status of `rm`, regardless of +# whether sqlite3 succeeded). Steps: index a real fixture repo; drop the +# `symbols` table so _ctx_generate_file_sql's own INSERT statements fail; +# call _ctx_index_file directly (white-box); assert it returns non-zero. +case_ctx_index_file_return_code_reflects_sqlite_failure() { + local name="pmctl context: _ctx_index_file's return code reflects the actual sqlite3 result, not rm's" + should_run "$name" || return 0 + + local fix_repo="$tmp_root/fix-repo-index-file-rc" + make_fixture_repo "$fix_repo" + local err="$tmp_root/index-file-rc-setup.err" + "$PMCTL" context index "$fix_repo" > /dev/null 2> "$err" \ + || { fail "$name" "setup: initial index failed: $(<"$err")"; return 0; } + + local db="$fix_repo/.pm-dispatch/ctx/context.db" + local target="$fix_repo/scripts/lib/mymodule.sh" + sqlite3 "$db" "DROP TABLE symbols;" 2>/dev/null \ + || { fail "$name" "setup: could not drop symbols table"; return 0; } + + local out + out="$tmp_root/index-file-rc.out" + bash -c ' + set -uo pipefail + # shellcheck source=runtime/lib/pmctl-context.sh + . "$1/lib/pmctl-context.sh" + _ctx_index_file "$2" "$3" "scripts/lib/mymodule.sh" + echo "rc=$?" + ' bash "$REPO_ROOT/runtime" "$(ctx_fixture_target "$db")" "$(ctx_fixture_target "$target")" \ + > "$out" 2>>"$err" || true + + local rc + rc="$(grep '^rc=' "$out" | cut -d= -f2)" + if [[ "$rc" -eq 0 ]]; then + fail "$name" "_ctx_index_file reported success (rc=0) despite the forced sqlite3 failure" + return 0 + fi + pass "$name" +} + +# Behavior (CC-571): unlike a failed FTS rebuild (best-effort, non-fatal), +# a failed _ctx_index_file writes the primary files/symbols/file_chunks +# data -- pmctl_context_update must treat it as fatal and must NOT print +# "context update: re-indexed " when the file's content was not +# actually reflected in the index. +case_context_update_fails_honestly_when_index_file_fails() { + local name="pmctl context update: fails (does not claim re-indexed) when _ctx_index_file fails" + should_run "$name" || return 0 + + local fix_repo="$tmp_root/fix-repo-update-index-file-fail" + make_fixture_repo "$fix_repo" + local err="$tmp_root/update-index-file-fail-setup.err" + "$PMCTL" context index "$fix_repo" > /dev/null 2> "$err" \ + || { fail "$name" "setup: initial index failed: $(<"$err")"; return 0; } + + local target="$fix_repo/scripts/lib/mymodule.sh" + local out status=0 + out="$tmp_root/update-index-file-fail.out" + bash -c ' + set -uo pipefail + # shellcheck source=runtime/lib/pmctl-context.sh + . "$1/lib/pmctl-context.sh" + _ctx_index_file() { return 1; } + pmctl_context_update "$2" "$3" + ' bash "$REPO_ROOT/runtime" "$(ctx_fixture_target "$fix_repo")" "$(ctx_fixture_target "$target")" \ + > "$out" 2> "$err" || status=$? + + if [[ "$status" -eq 0 ]]; then + fail "$name" "pmctl_context_update exited 0 despite a failed _ctx_index_file; expected non-zero. out=$(<"$out")" + return 0 + fi + if grep -q '^context update: re-indexed' "$out"; then + fail "$name" "claimed 're-indexed' success despite a failed _ctx_index_file: $(<"$out")" + return 0 + fi + if ! grep -q 'failed to index' "$err"; then + fail "$name" "expected an honest failure message on stderr; got: $(<"$err")" + return 0 + fi + pass "$name" +} + +# Behavior (CC-571): pmctl_context_index must not report a bare "context +# index: N indexed, M skipped" success line when the FTS rebuild it +# triggered actually failed -- the previous behavior silently ignored +# _ctx_fts_rebuild's return value entirely. Overall indexing must still +# succeed (FTS is a best-effort acceleration layer; LIKE fallback remains +# available), but stderr must honestly say the FTS index is now stale. +# Steps: shadow _ctx_fts_rebuild to always fail (white-box, matching this +# file's existing sqlite3-shadowing pattern); run pmctl_context_index on a +# fresh fixture repo (which will trigger an FTS rebuild since content_fts +# does not exist yet); assert exit 0 (non-fatal) and the honest stderr +# message. +case_context_index_reports_fts_rebuild_failure_honestly() { + local name="pmctl context index: reports (not silently absorbs) an FTS rebuild failure" + should_run "$name" || return 0 + + local fix_repo="$tmp_root/fix-repo-fts-index-honest" + make_fixture_repo "$fix_repo" + + local out err status=0 + out="$tmp_root/fts-index-honest.out"; err="$tmp_root/fts-index-honest.err" + bash -c ' + set -uo pipefail + # shellcheck source=runtime/lib/pmctl-context.sh + . "$1/lib/pmctl-context.sh" + _ctx_fts_rebuild() { return 1; } + pmctl_context_index "$2" + ' bash "$REPO_ROOT/runtime" "$(ctx_fixture_target "$fix_repo")" > "$out" 2> "$err" || status=$? + + if [[ "$status" -ne 0 ]]; then + fail "$name" "pmctl_context_index exited $status; expected 0 (FTS failure must be non-fatal). err=$(<"$err")" + return 0 + fi + if ! grep -q 'FTS index rebuild failed' "$err"; then + fail "$name" "expected an honest FTS-rebuild-failure message on stderr; got: $(<"$err")" + return 0 + fi + # gate finding critic-F001 (round 1): printing the diagnostic on stderr + # while stdout still read as an unqualified "N indexed, M skipped" is a + # contradictory summary for any caller that only looks at stdout / exit + # code. The final summary line itself must carry the degradation. + if ! grep -qE '^context index: .*degraded' "$out"; then + fail "$name" "expected the summary line itself to disclose FTS degradation; got: $(<"$out")" + return 0 + fi + # gate finding critic-F001 (round 2, gate-20260826-021038-ac0bc2): this is + # a fresh fixture repo with no prior content_fts, so the rollback after + # this failed rebuild leaves NO FTS table at all -- "existing (now stale) + # FTS index retained" would be false here, since there is no existing + # index to retain. Both messages must say no index is available instead. + if grep -qi 'existing.*retained' "$err" || grep -qi 'stale index retained' "$out"; then + fail "$name" "first-time build failure wrongly claimed a stale index is 'retained' when none ever existed. err=$(<"$err") out=$(<"$out")" + return 0 + fi + if ! grep -q 'no FTS index available' "$err"; then + fail "$name" "expected the first-time-build failure message to say no FTS index is available; got: $(<"$err")" + return 0 + fi + local db_after="$fix_repo/.pm-dispatch/ctx/context.db" + local fts_after + fts_after="$(sqlite3 "$db_after" "SELECT count(*) FROM sqlite_master WHERE type='table' AND name='content_fts';" 2>/dev/null)" + if [[ "$fts_after" != "0" ]]; then + fail "$name" "expected content_fts to be absent after a failed first-time rebuild; sqlite_master reports $fts_after" + return 0 + fi + pass "$name" +} + +# Behavior (CC-571): same honest-reporting contract as +# case_context_index_reports_fts_rebuild_failure_honestly, but for +# pmctl_context_update's independent call site -- both call sites had the +# identical unchecked-return-value gap before this fix, and CC-521's own +# lesson (grep every consumer, not just one) applies here. +case_context_update_reports_fts_rebuild_failure_honestly() { + local name="pmctl context update: reports (not silently absorbs) an FTS rebuild failure" + should_run "$name" || return 0 + + local fix_repo="$tmp_root/fix-repo-fts-update-honest" + make_fixture_repo "$fix_repo" + local err="$tmp_root/fts-update-honest-setup.err" + "$PMCTL" context index "$fix_repo" > /dev/null 2> "$err" \ + || { fail "$name" "setup: initial index failed: $(<"$err")"; return 0; } + + local target="$fix_repo/scripts/lib/mymodule.sh" + printf '\n# touched for CC-571 update-path test\n' >> "$target" + + local out status=0 + out="$tmp_root/fts-update-honest.out" + bash -c ' + set -uo pipefail + # shellcheck source=runtime/lib/pmctl-context.sh + . "$1/lib/pmctl-context.sh" + _ctx_fts_rebuild() { return 1; } + pmctl_context_update "$2" "$3" + ' bash "$REPO_ROOT/runtime" "$(ctx_fixture_target "$fix_repo")" "$(ctx_fixture_target "$target")" \ + > "$out" 2> "$err" || status=$? + + if [[ "$status" -ne 0 ]]; then + fail "$name" "pmctl_context_update exited $status; expected 0 (FTS failure must be non-fatal). err=$(<"$err")" + return 0 + fi + if ! grep -q 'FTS index rebuild failed' "$err"; then + fail "$name" "expected an honest FTS-rebuild-failure message on stderr; got: $(<"$err")" + return 0 + fi + # gate finding critic-F001 (round 1): same contradictory-summary concern + # as the index-path test above. + if ! grep -qE '^context update: re-indexed .*degraded' "$out"; then + fail "$name" "expected the summary line itself to disclose FTS degradation; got: $(<"$out")" + return 0 + fi + pass "$name" +} + # Behavior (CC-505 Req 2/3 gate finding qa-tester-F001): _ctx_rank_hits is the # ONE shared sort+truncate path every consumer (query/pack/reuse-scan/ # prompt-scan) calls. A comparator, tie-break, rank-numbering, or limit @@ -5523,6 +5785,11 @@ case_context_prompt_scan_no_sqlite_graceful case_context_prompt_scan_secret_never_persisted case_context_prompt_scan_emits_event case_context_fts5_availability_is_cached +case_ctx_fts_rebuild_rollback_preserves_old_index_on_failure +case_ctx_index_file_return_code_reflects_sqlite_failure +case_context_update_fails_honestly_when_index_file_fails +case_context_index_reports_fts_rebuild_failure_honestly +case_context_update_reports_fts_rebuild_failure_honestly case_context_rank_hits_orders_and_truncates case_context_pack_ranking_fields_are_valid case_context_pack_default_budget_does_not_truncate