chore(SESF-45): scrub pre-existing exception logs in rag_engine.py

[lbruton]

Summary

Routes 8 pre-existing logger.warning/error exception-log sites in rag_engine.py through _scrub_exception(). SESF-41 added the scrubber and SESF-42 scrubbed its own new sites; this closes the remaining pre-existing surface (a SQLite/Milvus error can occasionally carry a fragment of the offending value).

Sites: close_server_mode, _get_persistent_client (×2), add_turns dedup, get_issue_timeline FTS fallback, delete_by_session / delete_by_branch / delete_older_than.

Linked issue

• Plane SESF-45

Notes

• No behavior change beyond log-message scrubbing. grep confirms zero logger.{warning,error}(…, e) exception sites remain unscrubbed in rag_engine.py.
• Drafted during the SESF-42 review (the code-reviewer flagged these as out-of-scope siblings), stashed, and re-applied cleanly on current main.

Tests

• Full suite 360 passed; ruff clean. +test for scrubbed FTS exception logging.

[coderabbitai]

Warning

Review limit reached

@lbruton, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 14 minutes and 20 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c4b3c9ff-daea-47f6-abbb-bf23a59de617

📥 Commits

Reviewing files that changed from the base of the PR and between f0a35cd and 00618e7.

📒 Files selected for processing (2)

• rag_engine.py
• tests/test_secret_redaction.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/scrub-pre-existing-exception-logs

---

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

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	✅ 0 (≤ 500 complexity)
Duplication	✅ 0 (≤ 5 duplication)

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

The PR successfully implements exception log scrubbing across several components, but it introduces or overlooks critical resource management issues and security gaps.

Specifically, three methods (delete_by_session, delete_by_branch, and delete_older_than) fail to close FTS connections when exceptions occur, leading to SQLite file handle leaks. Additionally, while local logs are now scrubbed in _get_persistent_client, re-raising the original exception without modification allows sensitive data to leak into upstream loggers.

There are also significant gaps in test coverage for Milvus connection and deduplication exception paths which should be addressed to ensure the scrubbing logic is functional in those areas.

About this PR

• The Milvus connection and deduplication exception paths lack corresponding test cases to verify that exceptions are correctly routed through the _scrub_exception helper.

Test suggestions

• Verify FTS deletion errors are scrubbed in delete_by_session, delete_by_branch, and delete_older_than
• Verify Milvus connection and client-closing errors are scrubbed
• Verify errors during the document deduplication query are scrubbed
• Verify FTS fallback errors during issue timeline retrieval are scrubbed

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Verify Milvus connection and client-closing errors are scrubbed
2. Verify errors during the document deduplication query are scrubbed
3. Verify FTS fallback errors during issue timeline retrieval are scrubbed

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🔴 HIGH RISK}

The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. Use a finally block to ensure _fts.close_ephemeral(conn) is called regardless of success or failure.

[codacy-production]

_{🔴 HIGH RISK}

The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. Use a finally block to ensure _fts.close_ephemeral(conn) is called regardless of success or failure.

[codacy-production]

_{🔴 HIGH RISK}

The ephemeral FTS connection is not closed if an exception occurs during deletion, resulting in a resource leak. The block should use a finally clause to ensure the connection is always released. Refactor delete_by_session to ensure _fts.close_ephemeral(conn) is called in a finally block.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: While the local log is now scrubbed, re-raising the exception e without modification means any upstream logger will still record the sensitive data. Additionally, db_path may contain credentials that should be redacted before logging. Update the logging to redact the db_path argument and mutate e.args to redact secrets before re-raising.