fix(reliability): route session-response write through WAL helper, not inline sqlite (A-20)#64
Merged
Merged
Conversation
…t inline sqlite (A-20) Closes A-20 — codec.py's voice handler opened a bare sqlite3.connect(DB_PATH) for one `UPDATE sessions SET response=? WHERE id=?` with no WAL/busy_timeout, risking "database is locked" under concurrent Phase-3 agent-runner + voice writes. ## Correction over the audit's literal fix The audit suggested CodecMemory().update_session_response(), but the `sessions` table is owned by codec_core (CodecMemory owns `conversations`). So the helper lives in codec_core. ## What changed codec_core.py: - _db_connect() — connection helper setting PRAGMA journal_mode=WAL + busy_timeout=5000 (matches CodecMemory + routes/_shared.get_db()). - update_session_response(rid, response) — truncates to 500 chars, never raises (logs DB errors; the caller already spoke the answer). - Retrofitted ALL four real session connects (init_db, save_task, get_memory, get_recent_conversations) to use _db_connect() — they shared the same no-pragma lock risk. (String-template connects in the deprecated build_session_script untouched.) codec.py: - Replaced the inline sqlite UPDATE with update_session_response(rid, answer[:500]). - Removed now-unused `sqlite3` + `DB_PATH` imports (ruff 73 -> 72). Dashboard qchat_db/vibe_db lazy globals already set pragmas — left as-is per the audit. ## Verification 9 tests in tests/test_session_response_update.py (WAL+busy_timeout pragmas, round-trip, 500-char truncation, None/nonexistent rid no-crash, non-str coercion, surfaces in get_memory, source invariants). Full suite 1365 passing, 23 pre-existing failures unchanged (zero new). Net-negative ruff. No skills/ touched -> no manifest regen. Reference: docs/audits/PHASE-1-CODE-QUALITY.md finding A-20. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes A-20 (MEDIUM) — codec.py's voice handler opened a bare
sqlite3.connect(DB_PATH)for oneUPDATE sessions SET response=? WHERE id=?with no WAL/busy_timeout, riskingdatabase is lockedunder concurrent Phase-3 agent-runner + voice writes.Correction over the audit's literal fix
The audit suggested
CodecMemory().update_session_response(), but thesessionstable is owned bycodec_core(CodecMemory ownsconversations). So the helper lives incodec_core, where the table +save_taskalready are.What changed
codec_core.py:_db_connect()— connection helper settingPRAGMA journal_mode=WAL+busy_timeout=5000(matchesCodecMemory+routes/_shared.get_db()).update_session_response(rid, response)— truncates to 500 chars (prior behavior), never raises (logs DB errors; the caller already spoke the answer).init_db,save_task,get_memory,get_recent_conversations) to use_db_connect()— they shared the same no-pragma lock risk. (The string-template connects inside the deprecatedbuild_session_scriptare untouched.)codec.py:update_session_response(rid, answer[:500]).sqlite3+DB_PATHimports (ruff 73 → 72).The dashboard
qchat_db/vibe_dblazy globals already set pragmas — left as-is per the audit.Verification
9 tests in
tests/test_session_response_update.py: WAL+busy_timeout pragmas, round-trip write, 500-char truncation,None/nonexistent-rid no-crash, non-str coercion, surfaces inget_memory, source invariants (no inline UPDATE in codec.py; exactly one bare connect in codec_core — inside_db_connect).Full suite: 1365 passing, 23 pre-existing failures unchanged (zero new). Net-negative ruff. No
skills/touched → no manifest regen.Reference: docs/audits/PHASE-1-CODE-QUALITY.md finding A-20.
🤖 Generated with Claude Code