fix(CC-571): sqlite atomic-script gap in _ctx_fts_rebuild + _ctx_index_file

[screenleon]

Summary

_ctx_fts_rebuild's DROP+CREATE+INSERT sequence had no BEGIN/COMMIT, and critically no -bail: direct reproduction confirmed 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. Both callers (pmctl_context_index, pmctl_context_update) also called it unchecked, reporting success regardless.

Fixed by wrapping in BEGIN IMMEDIATE/COMMIT with sqlite3 -bail: an error now aborts before COMMIT, leaving the transaction open; the process exit closes the connection, triggering an automatic ROLLBACK that leaves the previous content_fts fully intact. Both callers now check the return value and report the degradation — including in the final stdout summary line itself, not just stderr (see gate findings below) — instead of an unqualified success message. Non-fatal, since FTS is a best-effort acceleration layer (LIKE fallback remains available).

Scope expanded during the mandatory reuse/simplify pass: the altitude review found the identical pattern in _ctx_index_file (pmctl_context_update's other call site), which is more severe since it writes the primary files/symbols/file_chunks data, not an acceleration layer. Direct testing also found a third, independent bug there: the function's trailing rm -f "$tmpf" masked sqlite3's real exit code (the function always returned rm's status, ~always 0, regardless of whether sqlite3 succeeded). Both functions now share one _ctx_sqlite_exec_atomic helper (also closing a reuse-review finding that the fix reinvented a pattern already established in memory.sh's memory_usage_commit rather than citing it), and _ctx_index_file's caller now treats its failure as fatal — unlike FTS, a failed primary-index write must not be reported as "re-indexed".

Gate findings fixed (2 rounds)

• critic-F001 (round 1): stderr carried the degradation diagnostic, but stdout still printed a bare "N indexed, M skipped" / "re-indexed " — a contradictory summary for any caller that only looks at stdout or exit code. Fixed by appending the degradation qualifier directly onto the stdout summary line.
• critic-F001 (round 2): when content_fts didn't exist before the rebuild attempt (first-time build), the message wrongly said "existing (now stale) FTS index retained" — there was no existing index to retain. Fixed by branching the message on whether content_fts existed beforehand.

Verification

• pmctl gate run --executor codex --policy generic — Final: GO (critic/qa-tester/architecture-reviewer/security-reviewer, tier standard, 5 rounds total — 2 real findings fixed, remaining rounds were gate-executor synthesis protocol instability unrelated to the code, diagnosed via supervisor-stdout.log).
• tests/shell/test-pmctl-context.sh — 168 passed, 0 failed (7 new/strengthened cases covering: FTS rollback preserves the old index, _ctx_index_file's return code reflects sqlite3's real result, both callers report failure honestly in their stdout summary, and the first-time-build-failure wording).
• tests/bin/run-tests.sh --base main — 9/9 diff-selected suites passed (incl. lint-scripts).
• tests/bin/run-all-tests.sh — 104 passed, 0 failed, 0 skipped (full sign-off).

🤖 Generated with Claude Code

https://claude.ai/code/session_0177ds9nxEMtsh3bwpBxNHYx