fix(dolt): align Beads shared server storage - #2167
Conversation
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughThe Dolt Home Manager module now stores beads data beneath ChangesDolt shared-server configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/dolt_start_spec.sh (1)
70-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftAssert evaluated configuration rather than source text.
These
grepchecks only prove that the literal assignments exist indefault.nix; they do not verify that Home Manager evaluates them or thatbdand launchd receive the same path. Prefer the repository’s Nix/Home Manager test harness, or add evaluated-value assertions plus a regression check that the old path is no longer used.🤖 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 `@spec/dolt_start_spec.sh` around lines 70 - 87, The tests in the “Describe home-manager/services/dolt/default.nix” block currently grep source text instead of validating evaluated Home Manager configuration. Replace these checks with the repository’s Nix/Home Manager test harness, asserting the evaluated sharedServerDir, beadsDir, and BEADS_SHARED_SERVER_DIR values are identical and verifying launchd/bd receive that path; also assert the obsolete path is absent.
🤖 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.
Nitpick comments:
In `@spec/dolt_start_spec.sh`:
- Around line 70-87: The tests in the “Describe
home-manager/services/dolt/default.nix” block currently grep source text instead
of validating evaluated Home Manager configuration. Replace these checks with
the repository’s Nix/Home Manager test harness, asserting the evaluated
sharedServerDir, beadsDir, and BEADS_SHARED_SERVER_DIR values are identical and
verifying launchd/bd receive that path; also assert the obsolete path is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b2156c67-2a8b-4f5c-b13e-1e956eb9108d
📒 Files selected for processing (2)
home-manager/services/dolt/default.nixspec/dolt_start_spec.sh
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
|
Addressed the review findings in
Revalidated: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/services/dolt/start.sh">
<violation number="1" location="home-manager/services/dolt/start.sh:21">
P2: A dangling symlink at a canonical database path makes Dolt fail to start instead of preserving the existing destination. Treat symlinks as occupied targets before invoking `mv`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| target_name="df" | ||
| fi | ||
|
|
||
| if [ -e "@beadsDir@/$target_name" ]; then |
There was a problem hiding this comment.
P2: A dangling symlink at a canonical database path makes Dolt fail to start instead of preserving the existing destination. Treat symlinks as occupied targets before invoking mv.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/dolt/start.sh, line 21:
<comment>A dangling symlink at a canonical database path makes Dolt fail to start instead of preserving the existing destination. Treat symlinks as occupied targets before invoking `mv`.</comment>
<file context>
@@ -1,15 +1,29 @@
+ target_name="df"
+ fi
+
+ if [ -e "@beadsDir@/$target_name" ]; then
+ continue
+ fi
</file context>
| if [ -e "@beadsDir@/$target_name" ]; then | |
| if [ -e "@beadsDir@/$target_name" ] || [ -L "@beadsDir@/$target_name" ]; then |
1 participant
Summary
BEADS_SHARED_SERVER_DIRsobdand launchd resolve the same storageValidation
shellspec spec/dolt_start_spec.sh(14 examples, 0 failures)make nix-format-checkmake buildbd list,bd doctor --server --agent --json,bd dolt pull, andbd dolt pushagainst the repaired v59 databasesActivation note
make switchactivated the Nix configuration and restarted Dolt successfully. The enclosing target later failed in the unrelateddotagents-syncstep because that project has nosyncmake target.Summary by cubic
Align Dolt’s data directory with Beads’ shared-server storage and migrate safely from repo-local databases. Ensures
bdandlaunchdshare the same data.~/.beads/shared-server/dolt.BEADS_SHARED_SERVER_DIRsobdandlaunchdresolve the same path.~/dotfiles/.beads(skip symlinks, require.dolt, don’t overwrite; mapdolt→dfduring migration only).shellspecfor migration and path config; use a portable fakedoltand mark Nix literals to avoid ShellCheck noise.Written for commit b44b899. Summary will update on new commits.