test(server): isolate task tool SQLite fixtures - #199
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughВ ChangesТестовая SQLite-база
Estimated code review effort: 1 (Trivial) | ~2 минуты Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review focus:test isolation correctness and regression risk files:pkg/server/task_tool_test.go |
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the SQLite database connection string in the test helper to use the test name instead of a timestamp. The review feedback correctly points out that using the test name directly can cause failures in subtests because subtest names contain slashes, which SQLite interprets as directory paths. A suggestion was provided to sanitize the test name by replacing slashes with underscores.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Focused file outline =="
ast-grep outline pkg/server/task_tool_test.go --items all || true
echo
echo "== Pull request diff =="
git diff --unified=80 HEAD^ HEAD -- pkg/server/task_tool_test.go
echo
echo "== Fixture helper definitions and call sites =="
rg -n -C 8 '\bnewTaskToolEngine\s*\(' pkg/server/task_tool_test.go
echo
echo "== Test and subtest naming around the focused helper =="
rg -n -C 5 '^\s*func\s+Test|t\.Run\s*\(' pkg/server/task_tool_test.go
echo
echo "== SQLite connection / in-memory URI construction in focused file =="
rg -n -C 6 'sqlite|SQLite|:memory:|mode=memory|cache=shared|dsn|DSN|Open\s*\(' pkg/server/task_tool_test.goLength of output: 47566 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Count fixture-construction calls per test/helper function =="
python3 - <<'PY'
from pathlib import Path
import re
path = Path("pkg/server/task_tool_test.go")
lines = path.read_text().splitlines()
func = None
counts = {}
pattern = re.compile(r'\b(?:newTaskToolEngine|newTaskToolServer(?:WithProfile|WithWorkflowHooks)?)\s*\(')
for line in lines:
match = re.match(r'^func\s+([A-Za-z0-9_]+)\s*\(', line)
if match:
func = match.group(1)
counts.setdefault(func, 0)
if func and pattern.search(line) and not re.match(r'^func\s+(?:newTaskToolEngine|newTaskToolServer(?:WithProfile|WithWorkflowHooks)?)\s*\(', line):
counts[func] += 1
for name, count in counts.items():
if count:
print(f"{name}: {count}")
PY
echo
echo "== Locate and map loom engine construction =="
rg -n -C 5 '\bfunc\s+NewEngine\s*\(' loom
engine_file="$(rg -l --glob '*.go' '^\s*func\s+NewEngine\s*\(' loom | head -n 1)"
if [ -n "$engine_file" ]; then
ast-grep outline "$engine_file" --items all
echo
echo "== NewEngine implementation =="
start="$(rg -n '^\s*func\s+NewEngine\s*\(' "$engine_file" | head -n 1 | cut -d: -f1)"
sed -n "${start},$((start + 140))p" "$engine_file"
fi
echo
echo "== Other task-tool fixture database URI builders in this package =="
rg -n -C 4 'file:.*(?:memory|cache=shared)|mode=memory|task_tool_' pkg/server --glob '*_test.go'Length of output: 15223
No findings. ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pkg/server/task_tool_test.go`:
- Line 1718: Update the SQLite URI construction in the test setup around
sql.Open to escape t.Name() or replace it with a safe deterministic hash before
inserting it into the database URI. Preserve the shared in-memory database
behavior while ensuring names containing slashes, query characters, or spaces
cannot alter URI parsing or cause test databases to collide.
🪄 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: e4a0f0b6-cfea-46e1-88d4-d0fb30873271
📒 Files selected for processing (1)
pkg/server/task_tool_test.go
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review focus:verify SQLite URI escaping and test isolation files:pkg/server/task_tool_test.go incremental |
|
@codex review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Focused file outline =="
ast-grep outline pkg/server/task_tool_test.go --items all
echo
echo "== Incremental diff =="
git diff --unified=60 HEAD^ HEAD -- pkg/server/task_tool_test.go
echo
echo "== Fixture URI construction and lifecycle =="
rg -n -C 14 -P 'func\s+newTaskToolEngine\s*\(|t\.Name\(\)|sqlite|SQLite|mode=memory|cache=shared|Close\s*\(' pkg/server/task_tool_test.go
echo
echo "== Fixture call sites with enclosing test/subtest names =="
rg -n -C 8 -P '^\s*func\s+Test|t\.Run\s*\(|newTaskToolEngine\s*\(' pkg/server/task_tool_test.go
echo
echo "== SQLite driver module/version and URI-related references =="
rg -n -C 3 -P 'modernc\.org/sqlite|mattn/go-sqlite3|sqlite' go.mod go.sum pkg/server --glob '*.go'Length of output: 66700
No findings. ✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Use
t.Name()for the in-memory SQLite fixture URI innewTaskToolEngine, so each test invocation gets a stable test-local database instead of relying on wall-clock uniqueness under parallel/race scheduling.Evidence
go test ./pkg/server -run 'TestNewTaskToolEngine|TestLimitedRecipeProfile' -count=20 -timeout 300spkg/server/task_tool_test.go; no production behavior changes.Release context
This closes the independent package-test collision found while preparing v5.24.2. The muxcore lifecycle release blocker is tracked separately in Engram mcp-mux #405 and is not bypassed by this PR.
Summary by CodeRabbit