feat(database): update SQLite pool to create missing parent directories#230
Merged
Conversation
This commit enhances the SQLite connection pool to ensure that any missing parent directories for the database file are created during initialization. This change addresses a critical issue where the worker would crash on startup if the default database path did not exist. Additionally, it includes tests to verify the new behavior and updates the version to 0.2.7 in both `Cargo.toml` and `Cargo.lock` files.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Complex PR? Review this PR in Change Stack to move by importance, not file order. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR creates missing parent directories for file-backed SQLite pools (with tests and a version bump), adds a GitHub Actions ChangesSQLite Parent Directory Creation Fix
Interface Smoke Test CI Job
Harness Quickstart Documentation Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
skill-check — worker0 verified, 14 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 323-331: The workflow step "Dump logs on failure" uses the direct
expansion worker-${{ matrix.worker }} in the shell which risks template/shell
injection; change it to export or set an env var (e.g., WORKER_NAME = ${{
matrix.worker }}) at the job or step level and then reference the log file via
the env var in the tail command (tail -n 200 "worker-${WORKER_NAME}.log" or tail
-n 200 "worker-${WORKER_NAME}.log" with proper quoting) so the matrix.worker
value is not directly interpolated by the shell; update the step that contains
the tail invocation to use the new WORKER_NAME env var instead of `${{
matrix.worker }}`.
- Around line 235-247: The workflow sets VERSION but never uses it and only runs
iii --version without verifying it; update the step so the VERSION env is
actually passed to the installer (e.g., export or invoke install-iii.sh with the
version argument if the installer supports it) and replace the bare iii
--version call with an explicit verification that the output equals the expected
VERSION (failing the job if it does not). Target the run block that downloads
and executes install-iii.sh and the final iii --version check so you both pass
VERSION to the installer and assert the installed iii version matches VERSION.
In `@database/tests/integration.rs`:
- Around line 216-221: AppState construction in the test is missing the required
config field; update the struct literal to include config (e.g., config:
build_state().config or otherwise obtain the same config used by build_state())
so the test AppState matches the one produced by build_state() and handlers that
read AppState.config will compile and run correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80844dcf-7160-42f1-83dd-5757091486f8
⛔ Files ignored due to path filters (1)
database/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/ci.ymldatabase/Cargo.tomldatabase/src/pool/sqlite.rsdatabase/tests/integration.rsharness/README.md
Comment on lines
+235
to
+247
| - name: Install iii CLI + engine | ||
| env: | ||
| VERSION: '0.17.0' | ||
| run: | | ||
| set -euo pipefail | ||
| curl -fsSL https://install.iii.dev/iii/main/install.sh -o /tmp/install-iii.sh | ||
| sh /tmp/install-iii.sh | ||
| { | ||
| echo "$HOME/.local/bin" | ||
| echo "$HOME/.iii/bin" | ||
| } >> "$GITHUB_PATH" | ||
| export PATH="$HOME/.local/bin:$HOME/.iii/bin:$PATH" | ||
| iii --version |
There was a problem hiding this comment.
VERSION env var is unused; installed version isn't verified.
The VERSION: '0.17.0' environment variable is defined but never passed to the install script. Additionally, iii --version is called without asserting the output matches the expected version, so the job could silently run with a different CLI version if the remote installer behavior changes.
Consider passing the version to the installer (if supported) or verifying the installed version:
🔧 Suggested fix to verify installed version
- name: Install iii CLI + engine
env:
VERSION: '0.17.0'
run: |
set -euo pipefail
curl -fsSL https://install.iii.dev/iii/main/install.sh -o /tmp/install-iii.sh
sh /tmp/install-iii.sh
{
echo "$HOME/.local/bin"
echo "$HOME/.iii/bin"
} >> "$GITHUB_PATH"
export PATH="$HOME/.local/bin:$HOME/.iii/bin:$PATH"
- iii --version
+ installed_version=$(iii --version | head -1)
+ echo "Installed: $installed_version (expected: $VERSION)"
+ if ! echo "$installed_version" | grep -qF "$VERSION"; then
+ echo "::warning::iii version mismatch: expected $VERSION, got $installed_version"
+ fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 235 - 247, The workflow sets VERSION
but never uses it and only runs iii --version without verifying it; update the
step so the VERSION env is actually passed to the installer (e.g., export or
invoke install-iii.sh with the version argument if the installer supports it)
and replace the bare iii --version call with an explicit verification that the
output equals the expected VERSION (failing the job if it does not). Target the
run block that downloads and executes install-iii.sh and the final iii --version
check so you both pass VERSION to the installer and assert the installed iii
version matches VERSION.
Comment on lines
+323
to
+331
| - name: Dump logs on failure | ||
| if: failure() | ||
| run: | | ||
| echo "::group::iii engine log" | ||
| tail -n 200 iii-engine.log || true | ||
| echo "::endgroup::" | ||
| echo "::group::worker log" | ||
| tail -n 200 "worker-${{ matrix.worker }}.log" || true | ||
| echo "::endgroup::" |
There was a problem hiding this comment.
Potential template injection: use env var instead of direct expansion.
Line 330 uses ${{ matrix.worker }} directly in a shell string. While matrix.worker is derived from directory names (which limits exploitation), the safer pattern used elsewhere in this job (e.g., line 288) is to assign it to an env var first. This avoids shell metacharacter interpretation if a worker directory name ever contains special characters.
🔒 Suggested fix
- name: Dump logs on failure
if: failure()
+ env:
+ WORKER: ${{ matrix.worker }}
run: |
echo "::group::iii engine log"
tail -n 200 iii-engine.log || true
echo "::endgroup::"
echo "::group::worker log"
- tail -n 200 "worker-${{ matrix.worker }}.log" || true
+ tail -n 200 "worker-$WORKER.log" || true
echo "::endgroup::"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Dump logs on failure | |
| if: failure() | |
| run: | | |
| echo "::group::iii engine log" | |
| tail -n 200 iii-engine.log || true | |
| echo "::endgroup::" | |
| echo "::group::worker log" | |
| tail -n 200 "worker-${{ matrix.worker }}.log" || true | |
| echo "::endgroup::" | |
| - name: Dump logs on failure | |
| if: failure() | |
| env: | |
| WORKER: ${{ matrix.worker }} | |
| run: | | |
| echo "::group::iii engine log" | |
| tail -n 200 iii-engine.log || true | |
| echo "::endgroup::" | |
| echo "::group::worker log" | |
| tail -n 200 "worker-$WORKER.log" || true | |
| echo "::endgroup::" |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 330-330: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 323 - 331, The workflow step "Dump
logs on failure" uses the direct expansion worker-${{ matrix.worker }} in the
shell which risks template/shell injection; change it to export or set an env
var (e.g., WORKER_NAME = ${{ matrix.worker }}) at the job or step level and then
reference the log file via the env var in the tail command (tail -n 200
"worker-${WORKER_NAME}.log" or tail -n 200 "worker-${WORKER_NAME}.log" with
proper quoting) so the matrix.worker value is not directly interpolated by the
shell; update the step that contains the tail invocation to use the new
WORKER_NAME env var instead of `${{ matrix.worker }}`.
The new SQLite parent-dir regression test omitted the required config field, causing clippy and E2E harness jobs to fail at compile time. Co-authored-by: Cursor <cursoragent@cursor.com>
andersonleal
approved these changes
Jun 5, 2026
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
2 participants
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.
This commit enhances the SQLite connection pool to ensure that any missing parent directories for the database file are created during initialization. This change addresses a critical issue where the worker would crash on startup if the default database path did not exist. Additionally, it includes tests to verify the new behavior and updates the version to 0.2.7 in both
Cargo.tomlandCargo.lockfiles.Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores