Fix Beads sync to use --flush-only flag to prevent data loss#316
Merged
Conversation
bd sync (without --flush-only) triggers git pull internally, which silently destroys locally-created beads when the branch has no upstream tracking ref. This matches the safe pattern already used by the beads-rust tracker (br). Fixes #314 https://claude.ai/code/session_01HWPpSnncVC4cvKjCpHBLeU
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
WalkthroughThe Beads tracker plugin's sync now runs Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (33.33%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #316 +/- ##
==========================================
+ Coverage 44.76% 44.93% +0.17%
==========================================
Files 97 97
Lines 30543 30546 +3
==========================================
+ Hits 13673 13727 +54
+ Misses 16870 16819 -51
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/content/docs/plugins/trackers/beads.mdx (1)
296-306:⚠️ Potential issue | 🟡 MinorStale "Automatic" label in the comparison table after this change.
Line 300 still describes Beads' git sync as "Automatic", but this PR explicitly moves all git operations (add / commit / push) to a manual user step. Only the JSONL flush is now automatic. The table entry is now misleading for users choosing between trackers.
📝 Suggested fix
-| Git sync | Manual | Automatic | +| Git sync | Manual | JSONL auto, git manual |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/content/docs/plugins/trackers/beads.mdx` around lines 296 - 306, The "Git sync" comparison row currently reads "Git sync | Manual | Automatic" and is stale; update that table row so Beads' column reflects manual git operations (e.g., change "Git sync | Manual | Automatic" to "Git sync | Manual | Manual" or "Git sync | Manual (manual add/commit/push)" ) to accurately represent that only JSONL flushing remains automatic and all git add/commit/push for Beads are manual.
🧹 Nitpick comments (1)
src/plugins/trackers/builtin/beads/index.ts (1)
486-491: Address non-evergreen comment reference in the sync method.The
--flush-onlyflag is a valid and confirmed feature (documented in community Beads guides for exporting to JSONL while skipping git operations), but the comment on line 489 references an external GitHub issue URL which violates the coding guideline "Avoid temporal context in comments; make them evergreen." Replace the issue reference with a self-contained description of the failure mode:Suggested change
- // Export tracker state to JSONL only. Use --flush-only to avoid git - // operations (pull/push) which can silently destroy locally-created beads - // when the branch has no upstream tracking ref. - // See: https://github.com/subsy/ralph-tui/issues/314 + // Export tracker state to JSONL only. Use --flush-only to avoid git + // operations (pull/push) which can silently destroy locally-created beads + // on branches that have no upstream tracking ref (i.e. not yet pushed).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/trackers/builtin/beads/index.ts` around lines 486 - 491, The comment above the execBd(['sync', '--flush-only']) call references an external GitHub issue; replace that temporal link with a concise, self-contained explanation: state that the '--flush-only' flag exports tracker state to JSONL and skips git operations (pull/push) because performing pull/push when the current branch lacks an upstream can silently overwrite or lose locally-created beads; update the comment near execBd/ sync/ '--flush-only' to document this failure mode and rationale without external links.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@website/content/docs/plugins/trackers/beads.mdx`:
- Around line 296-306: The "Git sync" comparison row currently reads "Git sync |
Manual | Automatic" and is stale; update that table row so Beads' column
reflects manual git operations (e.g., change "Git sync | Manual | Automatic" to
"Git sync | Manual | Manual" or "Git sync | Manual (manual add/commit/push)" )
to accurately represent that only JSONL flushing remains automatic and all git
add/commit/push for Beads are manual.
---
Nitpick comments:
In `@src/plugins/trackers/builtin/beads/index.ts`:
- Around line 486-491: The comment above the execBd(['sync', '--flush-only'])
call references an external GitHub issue; replace that temporal link with a
concise, self-contained explanation: state that the '--flush-only' flag exports
tracker state to JSONL and skips git operations (pull/push) because performing
pull/push when the current branch lacks an upstream can silently overwrite or
lose locally-created beads; update the comment near execBd/ sync/ '--flush-only'
to document this failure mode and rationale without external links.
…age) 67 tests covering sync, detection, getTasks, getTask, completeTask, updateTaskStatus, getNextTask, getEpics, isComplete, getPrdContext, getTemplate, and factory. Includes explicit test that sync uses --flush-only flag (issue #314 regression guard). https://claude.ai/code/session_01HWPpSnncVC4cvKjCpHBLeU
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/trackers/builtin/beads/index.test.ts (1)
19-19:mockReadFilePathsis captured but never asserted.This array is populated in the
readFilemock (line 101) but no test ever reads it. Consider either adding assertions that verify the correct file paths are passed toreadFile(e.g. in thegetPrdContexttests), or removing it to reduce dead code in the test infrastructure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/trackers/builtin/beads/index.test.ts` at line 19, The test defines mockReadFilePaths and populates it in the readFile mock but never asserts it; either remove mockReadFilePaths to eliminate dead test state or add assertions in the relevant tests (e.g., the getPrdContext tests) to verify the expected file paths were requested. Locate the readFile mock in this test file and either delete the mockReadFilePaths declaration and its push usage, or add assertions that compare mockReadFilePaths to the expected array of paths after calling getPrdContext (or other functions under test) so the mock is actually validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/plugins/trackers/builtin/beads/index.test.ts`:
- Line 19: The test defines mockReadFilePaths and populates it in the readFile
mock but never asserts it; either remove mockReadFilePaths to eliminate dead
test state or add assertions in the relevant tests (e.g., the getPrdContext
tests) to verify the expected file paths were requested. Locate the readFile
mock in this test file and either delete the mockReadFilePaths declaration and
its push usage, or add assertions that compare mockReadFilePaths to the expected
array of paths after calling getPrdContext (or other functions under test) so
the mock is actually validated.
The new beads/index.test.ts (67 tests, 91% coverage) was not being picked up by CI because test batches are explicit. Add it as an isolated batch (mocks node:child_process and node:fs) and include its lcov in the Codecov upload. https://claude.ai/code/session_01HWPpSnncVC4cvKjCpHBLeU
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.
Summary
Updated the Beads tracker plugin to use
bd sync --flush-onlyinstead ofbd syncto prevent data loss when syncing on branches without upstream tracking references.Key Changes
BeadsTrackerPlugin.sync()to callbd sync --flush-onlyinstead ofbd sync--flush-onlyis necessary to avoid silent data destruction--flush-onlyflaggit add .beads/ && git commit -m "Update beads" && git push)Implementation Details
The
--flush-onlyflag ensures that only the tracker state is exported to JSONL without performing git pull/push operations. This prevents a critical issue wherebd synccould silently destroy locally-created beads when the current branch has no upstream tracking reference (see issue #314).The change maintains the same sync interface while making the operation safer by:
https://claude.ai/code/session_01HWPpSnncVC4cvKjCpHBLeU
Summary by CodeRabbit
Bug Fixes
Documentation
Tests