fix: beads tracker closes tasks more consistently and in the correct database based on config.#108
Conversation
- Use bd close instead of bd update --status=closed (supports --reason)
- Add --force flag to close tasks with open parent epics
- Add beadsDbPath template variable for agent bd commands
- Update templates to use bd close --db {{beadsDbPath}} --reason
- Fix 's' key to work when stopped/idle
|
@jesse-merhi is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughBead closure changed from an update-based flow to a dedicated Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant UI as RunApp (TUI)
participant Engine as Engine
participant IterMgr as IterationManager
User->>UI: Press 's'
alt status == ready
UI->>UI: set status -> running
UI->>Engine: onStart / startExecution()
else status == stopped or idle
UI->>Engine: continueExecution()
alt currentIteration >= maxIterations
UI->>Engine: addIterations(1)
Engine->>IterMgr: add iteration
IterMgr-->>Engine: added (ok)
Engine-->>UI: success
UI->>UI: set status -> running
UI->>Engine: continueExecution()
else
Engine-->>UI: continued
UI->>UI: set status -> running
end
end
Engine-->>User: execution resumes / runs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #108 +/- ##
==========================================
+ Coverage 40.36% 41.58% +1.22%
==========================================
Files 57 57
Lines 12626 12600 -26
==========================================
+ Hits 5096 5240 +144
+ Misses 7530 7360 -170
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/templates/prompts.ts`:
- Around line 148-156: The examples in the "Closing a Bead" section use the `bd
close` command but omit the `--db` flag, causing inconsistency with other
builtin templates; update both example lines that show `bd close [bead-id]
--reason "..."` (the example for closing with a descriptive reason and the
specific `bd close devtuneai-001` example) to include the `--db <dbname>` flag
(e.g., `bd close [bead-id] --db <dbname> --reason "..."`) so they match the
other templates and clarify which database is targeted.
- Around line 135-138: Update the prompt examples that show the bd close command
to include the --db flag to match the built-in templates; specifically, modify
the prompt text in src/templates/prompts.ts where the example shows "bd close
[bead-id] --reason ..." so it becomes "bd close [bead-id] --db {{beadsDbPath}}
--reason ..." (to mirror the usage in src/templates/builtin.ts), or
alternatively add a brief clarifying sentence in the same prompt block
explaining when --db is required (e.g., when running from a different
directory); ensure the change references the example command string that
contains "bd close" so both prompt and builtin templates remain consistent.
In `@src/tui/components/RunApp.tsx`:
- Around line 840-856: The callback that performs continue logic (the function
using engine.continueExecution(), engine.addIterations(), setStatus and
referencing currentIteration and maxIterations) has stale closure risk because
currentIteration and maxIterations are not in its useCallback dependency list;
update the useCallback dependencies to include currentIteration and
maxIterations (in addition to engine and setStatus or other existing deps) so
the handler always sees up-to-date iteration values and behaves correctly when
deciding to add an iteration or continue.
🧹 Nitpick comments (2)
src/tui/components/RunApp.tsx (1)
840-840: Redundant engine check.The
&& enginecondition is unnecessary sinceengineis a required prop (not optional inRunAppProps). This is a minor nit but could be removed for clarity.♻️ Suggested simplification
- } else if ((status === 'stopped' || status === 'idle') && engine) { + } else if (status === 'stopped' || status === 'idle') {src/templates/engine.ts (1)
260-269: Confirmbeads.dbis the correct filename for the beads database.The codebase uses only
beads.dbas the database filename; no other variants exist in the TypeScript files. The file correctly begins with the required ABOUTME comment. The implementation properly chains fallbacks through tracker options, config, and process defaults.Consider adding a brief inline comment explaining the fallback chain for maintainability:
const workingDir = (trackerOptions?.workingDir as string) ?? config.cwd ?? process.cwd(); // Falls back to tracker options, then config cwd, then process cwd
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/plugins/trackers/builtin/beads.tssrc/templates/builtin.tssrc/templates/defaults/beads-bv.hbssrc/templates/defaults/beads.hbssrc/templates/engine.tssrc/templates/prompts.tssrc/templates/types.tssrc/tui/components/RunApp.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with 'ABOUTME: '
Files:
src/templates/types.tssrc/tui/components/RunApp.tsxsrc/templates/engine.tssrc/templates/builtin.tssrc/templates/prompts.tssrc/plugins/trackers/builtin/beads.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Start all code files with a file-level JSDoc comment section explaining the file's purpose, prefixed with "ABOUTME: "
Files:
src/templates/types.tssrc/tui/components/RunApp.tsxsrc/templates/engine.tssrc/templates/builtin.tssrc/templates/prompts.tssrc/plugins/trackers/builtin/beads.ts
🧠 Learnings (2)
📚 Learning: 2026-01-13T12:30:04.930Z
Learnt from: CR
Repo: subsy/ralph-tui PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T12:30:04.930Z
Learning: When ending a work session, complete all mandatory steps: file issues for remaining work, run quality gates, update issue status, push to remote with `git push`, clean up, and verify all changes are committed and pushed
Applied to files:
src/templates/defaults/beads.hbssrc/templates/builtin.tssrc/templates/prompts.tssrc/templates/defaults/beads-bv.hbs
📚 Learning: 2026-01-13T12:29:45.887Z
Learnt from: CR
Repo: subsy/ralph-tui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T12:29:45.887Z
Learning: When ending a work session, complete all mandatory workflow steps: file issues, run quality gates, update issue status, push to remote, clean up, and verify
Applied to files:
src/templates/defaults/beads.hbssrc/templates/builtin.tssrc/templates/prompts.tssrc/templates/defaults/beads-bv.hbs
🧬 Code graph analysis (2)
src/tui/components/RunApp.tsx (1)
src/chat/engine.ts (1)
setStatus(113-122)
src/templates/engine.ts (1)
src/config/types.ts (1)
RalphConfig(194-233)
🔇 Additional comments (11)
src/plugins/trackers/builtin/beads.ts (2)
11-21: Import additions look correct.The new imports align with the types used in this file. The reordering maintains alphabetical consistency.
431-436: Reconcile unconditional--forcewith documented usage.Templates in the codebase show users closing beads with
bd close [id] --reason "..."without the--forceflag. However, this method unconditionally uses--force. Without tests or documentation explaining why--forceis mandatory here, it's unclear whether this difference is intentional or if it masks legitimate error cases (e.g., tasks with incomplete dependencies).Either document why
--forceis essential for this workflow, or test both code paths to ensure tasks fail appropriately when they shouldn't be closed.src/templates/defaults/beads-bv.hbs (1)
13-13: Template updates align with the newbd closeworkflow.The addition of
{{beadsDbPath}}documentation and its usage in the close command instruction is consistent with the PR objectives. The template now correctly directs users to usebd closewith--dband--reasonflags.Also applies to: 49-49
src/templates/defaults/beads.hbs (1)
13-13: Template updates are consistent withbeads-bv.hbs.The changes correctly add the
{{beadsDbPath}}variable documentation and update the close instruction to usebd closewith the database path and reason flags.Also applies to: 39-39
src/templates/types.ts (1)
73-75: Type definition correctly extendsTemplateVariables.The new
beadsDbPathproperty is properly typed asstringwith clear documentation. This enables type-safe usage in templates and the template engine.src/templates/engine.ts (1)
256-256: LGTM!Clean integration of the new
beadsDbPathvariable into the template context.src/templates/builtin.ts (2)
72-72: LGTM!The template correctly uses
bd closewith--db {{beadsDbPath}}and--reason, aligning with the PR objective to target the correct database directory.
117-117: LGTM!Consistent with the BEADS_TEMPLATE change. Both beads templates now use the same
bd close --db {{beadsDbPath}} --reasonpattern.src/templates/prompts.ts (3)
166-170: LGTM!The browser testing example correctly shows the updated
bd closesyntax with--reason.
174-174: LGTM!Stop condition correctly references the new
bd closecommand.
193-193: Command reference updated correctly.The command reference section now shows
bd close [bead-id] --reason "...", which aligns with the new workflow. Consider whether to add the--dbflag here as well for completeness.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Fix stale closure bug: add currentIteration and maxIterations to useCallback dependency array in RunApp.tsx 's' key handler - Add --db flag documentation to prompts.ts for consistency with builtin templates - Correct misleading comment about --force flag (it's for pinned issues, not epic relationships) - Remove redundant engine null check (engine is a required prop)
|
Thanks for this contribution @jesse-merhi! The core changes here are solid - switching from I've pushed a follow-up commit (6200d47) addressing the code review feedback:
Note - The |
- Add engine.test.ts with tests for buildTemplateVariables and beadsDbPath computation (5 test cases covering all fallback paths) - Add beads.test.ts with tests for completeTask verifying --force flag and --reason handling (4 test cases) These tests improve patch coverage for the PR changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
fix: beads tracker closes tasks more consistently and in the correct database based on config.
bd closeinstead ofbd update --status=closed-> (supports --reason)Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.