fix(db): release operation-owned connections

[seonghobae]

Root cause

Package-owned PostgreSQL/session resources could survive until interpreter cleanup across orchestrator preparation, one-shot CLI commands, remote credential stores, and partially initialized config/secret stores. The same authority boundary also needed fail-closed DSN, stored-secret decode/decrypt, token-buffer, and JSONL accumulator resource-limit semantics before lower-layer work begins.

Bounded implementation

• validate TokenCounter configuration before extension/session acquisition and provide idempotent close;
• require buffer_percentage to be an exact integer from 0 through 50 before PostgreSQL/pg_tiktoken acquisition;
• require BatchAccumulator.max_records and .max_bytes to be exact positive integers after explicit/configured selection; explicit zero is not omission and malformed configured defaults fail closed before accumulation;
• close orchestrator and CLI-owned collaborators in reverse ownership order on success/failure;
• clean up PostgresConfigStore/SecretStore connections acquired before constructor failure;
• require explicit nonblank store DSNs before libpq target selection without rewriting valid DSNs;
• fail closed if Fernet was requested but cryptography is unavailable;
• keep no-key local/dev persistence strict Base64 + strict UTF-8; wrong-key/encrypted-secret failures remain bounded and do not retain secret material/cause/context;
• no schema, provider-request, model, release-authority, or cross-service contract change.

The branch contains the test-first RED→GREEN history for token-buffer validation, accumulator record/byte ceilings, connection ownership, DSN validation, and secret decode/decrypt boundaries. Predecessor evidence is development provenance only and does not transfer after a head change.

Current exact state

• Head: f366d03ae18c91050b4688c4f442f03a7839e286.
• Independently resolved protected main: bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.
• GitHub reports Draft and mergeable.
• CI 31363690716: completed / success.
• Security Scan 31363690712: completed / success.
• SAST Semgrep 31363690738: completed / success.
• Formal review evidence returned is COMMENTED, not approval.
• Returned GHAS/Semgrep inline findings are resolved and outdated; unresolved inline threads: zero.

These PR-triggered workflows still predate protected-main exact-source governance #88, so terminal-success staged runs are not final source-head acceptance.

Integration boundary

#86 and #89 independently touch neighboring configuration/bootstrap authority. After #88/protected prerequisites integrate, reconcile the exact protected results explicitly rather than choosing conflict sides. Preserve #86 typed/cache semantics, #87 ownership/store-DSN/secret-decode/token-resource semantics, and #89 source-precedence/type semantics with fresh regressions/evidence. Issue #90 CLI cancellation remains blocked while this resource-ownership branch and #85 are open.

Follow-up boundaries

Fresh protected-main inspection found three separate authority/privacy gaps that this mature branch must not absorb late:

1. Issue [Security/Operability] Separate pg_tiktoken installation from runtime token counting #126 — pg_tiktoken extension authority. TokenCounter._ensure_pg_tiktoken() performs CREATE EXTENSION IF NOT EXISTS pg_tiktoken on the ordinary runtime connection. After fix(db): release operation-owned connections #87 integrates or is superseded, [Security/Operability] Separate pg_tiktoken installation from runtime token counting #126 must move installation to reviewed provisioning/migration authority and leave runtime token counting with read-only capability verification, preserving validation-before-acquisition and deterministic-close behavior.
2. Issue [Data Integrity] Make orchestrator batch-key authority exact and fail-closed #129 — orchestrator batch-key authority. PostgresBatchOrchestrator._resolve_batch_uuid() currently attempts uuid.UUID(str(batch_key)) and, on parse failure, sends the original object to the llm_batches.input_file_path lookup. Arbitrary stringifiable non-string values can therefore influence batch identity or reach Psycopg/database work before an exact public selector boundary. After fix(db): release operation-owned connections #87 integrates or is superseded, [Data Integrity] Make orchestrator batch-key authority exact and fail-closed #129 must add realistic RED tests and require an exact accepted batch-key type/source policy before database I/O while preserving deliberate UUID-string and input-file-path selection.
3. Issue [Privacy] Bound token-counting database failure diagnostics #131 — token-counting diagnostic confidentiality. Protected main logs the generic PostgreSQL token-counting exception object directly at debug level. PostgreSQL/Psycopg primary error messages can contain rejected values, so this is not a provably content-free diagnostic path and the adjacent nosemgrep confidentiality assertion is too strong. After fix(db): release operation-owned connections #87 integrates or is superseded, [Privacy] Bound token-counting database failure diagnostics #131 must replace lower-layer exception rendering with a finite package-owned diagnostic category, retain the separate UndefinedFunction availability path, preserve no-fallback token authority, and prove with a secret-sentinel RED test that exception text no longer enters logs.

None of these follow-ups inherits #87 checks/reviews automatically. They must start from the protected result and reacquire fresh coverage/security/exact-source evidence.

Merge boundary

Keep Draft until #88 reaches protected main and the read-only central review-evidence prerequisite materially changes. Then reconcile only for integration correctness and regenerate every applicable exact-source CI/security/dependency/package/provenance/semantic-review/branch-policy gate. Merge only with zero valid unresolved findings and every review/approval requirement actually imposed by live repository/CWL policy. The protected code-owner gate is disabled/on hold for the current solo-maintainer state and is not a universal approval requirement. No predecessor/stale/generated-merge/queued/pending/skipped/cancelled/absent/status-only/author-only/infrastructure/rate-limited evidence is final acceptance.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 095cb579-5ac1-4786-b518-4ea7537d6ef8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[seonghobae]

@coderabbitai review

[seonghobae]

Superseded after fresh protected-main review. The branch is 94 protected-main commits behind and bundles concerns now split into narrower authorities: protected #150 covers owned connection/resource cleanup and invalid configured ceilings; protected config/bootstrap/CLI hardening covers the overlapping typed/source/input boundaries; protected token-diagnostic work covers the confidentiality follow-up; and #156 is the current narrow replacement for the remaining explicit-Fernet-unavailable fail-closed boundary. No #87 checks/reviews transfer. Closing this stale composite branch prevents it from remaining an apparent writer/merge candidate; follow-up issues such as #126/#108 must start from the then-current protected result.