feat(tracker): Support for Beads-Rust-BV plugin#342
Merged
subsy merged 9 commits intosubsy:mainfrom Mar 13, 2026
Merged
Conversation
|
@cachemoney is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a composite BeadsRustBvTrackerPlugin that composes the existing beads-rust tracker and integrates the bv CLI for graph-aware task selection and triage; registers the tracker, adds a built-in template/type, docs, and tests; exposes BV availability and triage controls. Changes
Sequence DiagramsequenceDiagram
participant User as User/Ralph
participant Plugin as BeadsRustBvTracker
participant BR as br CLI
participant BV as bv CLI
participant Cache as Triage Cache
User->>Plugin: getNextTask()
Plugin->>BR: request candidate tasks (delegate)
BR-->>Plugin: candidate tasks
alt bv available
Plugin->>Cache: check triage cache
alt cache missing/stale
Plugin->>BV: bv --robot-triage
BV-->>Plugin: triage recommendations
Plugin->>Cache: store triage
end
Plugin->>BV: bv --robot-next (with labels/epic filters)
alt BV returns task
BV-->>Plugin: selected task + metadata
Plugin-->>User: decorated task (bvScore/bvReasons/bvUnblocks)
else BV fails/empty
Plugin->>BR: fallback to delegate selection
BR-->>Plugin: fallback task
Plugin-->>User: task
end
else bv unavailable
Plugin->>BR: delegate selection
BR-->>Plugin: task
Plugin-->>User: task
end
User->>Plugin: completeTask()/updateTaskStatus()
Plugin->>Plugin: schedule triage refresh (throttled)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/templates/engine.ts (1)
648-654:⚠️ Potential issue | 🟡 Minor
installBuiltinTemplatesincludesbeads-rust-bvbut is missingbeads-rust.The
BEADS_RUST_TEMPLATEis imported at line 22 and handled ingetBuiltinTemplate, but it's absent from thetemplatesmap here. This meansralphwill install the beads-rust-bv template globally but not the beads-rust template. This appears to be a pre-existing omission, but since this PR is adding to this map, it's a good time to fix it.💡 Proposed fix
const templates: Record<string, string> = { 'default': DEFAULT_TEMPLATE, 'beads': BEADS_TEMPLATE, 'beads-bv': BEADS_BV_TEMPLATE, + 'beads-rust': BEADS_RUST_TEMPLATE, 'beads-rust-bv': BEADS_RUST_BV_TEMPLATE, 'json': JSON_TEMPLATE, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/templates/engine.ts` around lines 648 - 654, The templates map is missing the 'beads-rust' entry so installBuiltinTemplates won't install BEADS_RUST_TEMPLATE; update the templates Record named templates (in src/templates/engine.ts) to include the 'beads-rust': BEADS_RUST_TEMPLATE key/value alongside the existing entries (e.g., 'beads-rust-bv': BEADS_RUST_BV_TEMPLATE) so that getBuiltinTemplate/ installBuiltinTemplates can locate and install the beads-rust template.
🧹 Nitpick comments (5)
src/plugins/trackers/builtin/beads-rust-bv/index.ts (2)
84-115: Missing timeout onexecBv— risk of indefinite hang.If the
bvprocess hangs (e.g. waiting on a lock, network, or broken pipe), this promise never resolves, stalling the entire plugin. Consider adding a timeout that kills the child process after a reasonable deadline.💡 Proposed fix — add a timeout to execBv
async function execBv( args: string[], - cwd?: string + cwd?: string, + timeoutMs = 30_000 ): Promise<{ stdout: string; stderr: string; exitCode: number }> { return new Promise((resolve) => { const proc = spawn('bv', args, { cwd, env: { ...process.env }, stdio: ['ignore', 'pipe', 'pipe'], }); let stdout = ''; let stderr = ''; + let settled = false; + + const timer = setTimeout(() => { + if (!settled) { + settled = true; + proc.kill('SIGTERM'); + resolve({ stdout, stderr: stderr + '\nexecBv timed out', exitCode: 1 }); + } + }, timeoutMs); proc.stdout.on('data', (data: Buffer) => { stdout += data.toString(); }); proc.stderr.on('data', (data: Buffer) => { stderr += data.toString(); }); proc.on('close', (code) => { - resolve({ stdout, stderr, exitCode: code ?? 1 }); + if (!settled) { + settled = true; + clearTimeout(timer); + resolve({ stdout, stderr, exitCode: code ?? 1 }); + } }); proc.on('error', (err) => { - stderr += err.message; - resolve({ stdout, stderr, exitCode: 1 }); + if (!settled) { + settled = true; + clearTimeout(timer); + stderr += err.message; + resolve({ stdout, stderr, exitCode: 1 }); + } }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts` around lines 84 - 115, The execBv helper currently can hang if the spawned bv process never exits; modify execBv to start a timer (e.g., const timer = setTimeout(...)) after spawn that kills proc (proc.kill()) and appends a timeout message to stderr then resolves with a non-zero exitCode, and ensure you clearTimeout(timer) in both proc.on('close') and proc.on('error') handlers so the promise always resolves either on normal exit, error, or timeout; keep the existing stdout/stderr aggregation and preserve the proc/error handling semantics.
257-356:getNextTaskonly forwards the first label tobv --robot-next.Line 271 passes
labelsToUse[0]only. If the user specifies multiple labels (e.g."auth,backend"), subsequent labels are silently dropped. Ifbvsupports multiple--labelflags or comma-separated values, consider forwarding all labels. If this is a knownbvlimitation, a brief comment would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts` around lines 257 - 356, getNextTask currently only forwards the first label (labelsToUse[0]) to bv --robot-next, dropping additional labels; update the code that builds args (in getNextTask, where args is created before calling execBv) to pass all labels supported by bv—either by pushing each label as its own --label flag (e.g., loop over labelsToUse and args.push('--label', label)) or by joining labelsToUse into a single comma-separated value if bv expects that; if bv truly only accepts one label, add a short comment near labelsToUse explaining the limitation and consider logging/validating multi-label input.tests/plugins/beads-rust-bv-tracker.test.ts (3)
210-216: Extensiveas unknown ascasts for private member access are brittle.Nearly every test relies on
(plugin as unknown as {...})to reach internal state (bvAvailable,delegate,scheduleTriageRefresh,lastTriageOutput, etc.). If any of these property names are renamed in the implementation, these tests will silently compile but fail at runtime. Consider one of:
- Exposing a lightweight test-only interface or a
__testHarnessaccessor on the plugin.- Centralising the cast into a single typed helper (e.g.
asTestable(plugin)) so that at least there is a single place to update when internals change.Example helper to reduce cast sprawl
interface TestableBvPlugin { bvAvailable: boolean; delegate: { getNextTask: () => Promise<TrackerTask | undefined>; getTask: (id: string) => Promise<TrackerTask>; getTasks: () => Promise<TrackerTask[]>; completeTask: (id: string) => Promise<{ success: boolean; message: string }>; updateTaskStatus: (id: string, status: string) => Promise<TrackerTask | undefined>; validateSetup: (a: Record<string, unknown>) => Promise<string | null>; detect: () => Promise<{ available: boolean; brVersion: undefined }>; }; scheduleTriageRefresh: (force?: boolean) => void; lastTriageOutput: unknown; refreshTriage: () => Promise<void>; triageRefreshInFlight: Promise<void> | null; } function asTestable(plugin: BeadsRustBvTrackerPlugin): TestableBvPlugin { return plugin as unknown as TestableBvPlugin; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/beads-rust-bv-tracker.test.ts` around lines 210 - 216, Tests repeatedly use brittle "(plugin as unknown as {...})" casts to access private internals (bvAvailable, delegate, scheduleTriageRefresh, lastTriageOutput, etc.); centralize this by adding a single test-only accessor/helper and update tests to use it—create a Testable interface (e.g., TestableBvPlugin) describing the needed internals and implement an asTestable(plugin: BeadsRustBvTrackerPlugin): TestableBvPlugin helper that performs the cast in one place, then replace per-test casts (including in makePlugin) to call asTestable(plugin) to set or read internals like bvAvailable, delegate, scheduleTriageRefresh, refreshTriage, and lastTriageOutput.
42-56: Spawn mock discardsargs, limiting argument verification.The mock only receives
commandand ignores all arguments (args,options). This means tests cannot verify that the correct flags (e.g.--robot-next,--label backend) are being passed tobrorbv. This directly undermines the "forwards label filter to bv" test (line 335), which ends up assertingexpect(true).toBe(true)because it has no way to inspect the invocation.Consider capturing args alongside the command so that tests can assert on them:
Sketch of an args-aware mock
- spawn: (command: string) => { + spawn: (command: string, args?: string[]) => { const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; }; proc.stdout = new EventEmitter(); proc.stderr = new EventEmitter(); + + capturedSpawnCalls.push({ command, args: args ?? [] }); const matchIndex = spawnResponses.findIndex( (r) => r.command === command || r.command === '*'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/beads-rust-bv-tracker.test.ts` around lines 42 - 56, The spawn mock currently only accepts a single "command" string and discards "args" and "options", preventing tests from asserting flags; update the mock signature in the spawn implementation used in tests to accept (command: string, args?: string[], options?: any), capture and forward the args/options into the created proc object (or into the matched spawnResponses entries) so spawnResponses can be matched/inspected by both command and args; adjust the matching logic around spawnResponses.findIndex to consider r.command and r.args (or store the actual args on the returned response) so tests can assert on passed flags when calling spawn.
617-620: Polling loop timeout could be fragile under CI load.The 20 × 5ms (100ms) budget is tight for CI environments with constrained CPU. If this test becomes flaky, consider bumping the iteration count or sleep duration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/beads-rust-bv-tracker.test.ts` around lines 617 - 620, The polling loop that waits for refreshCalls === 2 and state.triageRefreshInFlight === null is too tight for CI; increase the allowed wait by changing the loop from 20×5ms to a larger budget (for example, 200 iterations with 10ms sleeps) or otherwise use a test utility with a 2s timeout to wait for the condition (i.e., replace the loop around refreshCalls and state.triageRefreshInFlight with a longer loop or a waitFor that times out after ~2000ms).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts`:
- Around line 228-243: Protect against missing triage data by adding runtime
checks and validation: in the block that uses this.lastTriageOutput (inside
getTasks), verify that this.lastTriageOutput is defined and that
this.lastTriageOutput.triage &&
Array.isArray(this.lastTriageOutput.triage.recommendations) before building
recMap and iterating tasks; skip or treat as empty when absent. Also harden
refreshTriage (the method that parses stdout into BvTriageOutput) to validate
the parsed object shape (ensure parsed.triage is an object and
parsed.triage.recommendations is an array) before assigning to
this.lastTriageOutput, and fall back to a safe default (e.g., undefined or an
object with empty recommendations) when validation fails so getTasks cannot
encounter undefined properties.
In `@tests/plugins/beads-rust-bv-tracker.test.ts`:
- Around line 1-9: Update the file-level JSDoc so the first line of the
top-of-file comment begins with the required prefix "ABOUTME: " (e.g., change
the existing comment intro to start with "ABOUTME: Tests for
BeadsRustBvTrackerPlugin..."); locate the file-level JSDoc at the top of the
test module (the header comment above the test declarations) and ensure the
prefix and brief purpose text are present exactly as required by the linting
guideline.
- Around line 335-353: The test title claims it forwards labels to bv but only
asserts true; update the test to actually assert spawn received the label flag:
when invoking plugin.getNextTask({ labels: ['backend'] }) have your spawn/mock
(the existing queueSpawnResponse or spawn interceptor) push the called command
and argv into capturedArgs, then assert that the bv invocation includes the
label flag (e.g. contains '--label' and 'backend' or a single '--label backend'
arg). Modify the test named "forwards label filter to bv" to use capturedArgs
and an expectation on capturedArgs for the bv command instead of
expect(true).toBe(true); keep plugin.getNextTask and queueSpawnResponse usage.
In `@website/content/docs/plugins/trackers/beads-rust-bv.mdx`:
- Around line 14-40: MDX v2 requires JSX tags that wrap block-level markdown to
have blank lines after the opening tag and before the closing tag; update every
<Step> element that contains a fenced code block (``` or ```bash) so there is an
empty line immediately after the opening <Step> and an empty line immediately
before the closing </Step> (e.g., for the <Step> blocks around the br init,
cargo install bv, and both version-check code fences, and all similar Steps in
the "Basic Usage" section) to resolve the "Expected a closing tag for <Step>"
compilation error.
---
Outside diff comments:
In `@src/templates/engine.ts`:
- Around line 648-654: The templates map is missing the 'beads-rust' entry so
installBuiltinTemplates won't install BEADS_RUST_TEMPLATE; update the templates
Record named templates (in src/templates/engine.ts) to include the 'beads-rust':
BEADS_RUST_TEMPLATE key/value alongside the existing entries (e.g.,
'beads-rust-bv': BEADS_RUST_BV_TEMPLATE) so that getBuiltinTemplate/
installBuiltinTemplates can locate and install the beads-rust template.
---
Nitpick comments:
In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts`:
- Around line 84-115: The execBv helper currently can hang if the spawned bv
process never exits; modify execBv to start a timer (e.g., const timer =
setTimeout(...)) after spawn that kills proc (proc.kill()) and appends a timeout
message to stderr then resolves with a non-zero exitCode, and ensure you
clearTimeout(timer) in both proc.on('close') and proc.on('error') handlers so
the promise always resolves either on normal exit, error, or timeout; keep the
existing stdout/stderr aggregation and preserve the proc/error handling
semantics.
- Around line 257-356: getNextTask currently only forwards the first label
(labelsToUse[0]) to bv --robot-next, dropping additional labels; update the code
that builds args (in getNextTask, where args is created before calling execBv)
to pass all labels supported by bv—either by pushing each label as its own
--label flag (e.g., loop over labelsToUse and args.push('--label', label)) or by
joining labelsToUse into a single comma-separated value if bv expects that; if
bv truly only accepts one label, add a short comment near labelsToUse explaining
the limitation and consider logging/validating multi-label input.
In `@tests/plugins/beads-rust-bv-tracker.test.ts`:
- Around line 210-216: Tests repeatedly use brittle "(plugin as unknown as
{...})" casts to access private internals (bvAvailable, delegate,
scheduleTriageRefresh, lastTriageOutput, etc.); centralize this by adding a
single test-only accessor/helper and update tests to use it—create a Testable
interface (e.g., TestableBvPlugin) describing the needed internals and implement
an asTestable(plugin: BeadsRustBvTrackerPlugin): TestableBvPlugin helper that
performs the cast in one place, then replace per-test casts (including in
makePlugin) to call asTestable(plugin) to set or read internals like
bvAvailable, delegate, scheduleTriageRefresh, refreshTriage, and
lastTriageOutput.
- Around line 42-56: The spawn mock currently only accepts a single "command"
string and discards "args" and "options", preventing tests from asserting flags;
update the mock signature in the spawn implementation used in tests to accept
(command: string, args?: string[], options?: any), capture and forward the
args/options into the created proc object (or into the matched spawnResponses
entries) so spawnResponses can be matched/inspected by both command and args;
adjust the matching logic around spawnResponses.findIndex to consider r.command
and r.args (or store the actual args on the returned response) so tests can
assert on passed flags when calling spawn.
- Around line 617-620: The polling loop that waits for refreshCalls === 2 and
state.triageRefreshInFlight === null is too tight for CI; increase the allowed
wait by changing the loop from 20×5ms to a larger budget (for example, 200
iterations with 10ms sleeps) or otherwise use a test utility with a 2s timeout
to wait for the condition (i.e., replace the loop around refreshCalls and
state.triageRefreshInFlight with a longer loop or a waitFor that times out after
~2000ms).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/plugins/trackers/builtin/beads-rust-bv/index.tssrc/plugins/trackers/builtin/index.tssrc/templates/builtin.tssrc/templates/engine.tssrc/templates/index.tssrc/templates/types.tstests/plugins/beads-rust-bv-tracker.test.tswebsite/content/docs/plugins/trackers/beads-rust-bv.mdxwebsite/lib/navigation.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts`:
- Around line 235-236: The loop that builds recMap from
this.lastTriageOutput.triage.recommendations should validate each entry before
using rec.id: update the block that iterates recommendations (and the similar
loop around the getTasks-related code) to skip or log any non-object or null
entries and any entries missing a string/number id, e.g., check typeof rec ===
'object' && rec !== null && ('id' in rec) before calling recMap.set(rec.id,
rec); ensure you perform the same defensive validation in the analogous code
handling recommendations at the 471-479 area so malformed array items cannot
cause runtime errors in getTasks.
- Around line 467-489: The debounce timestamp lastTriageRefreshAt is only set on
the successful-parse path, so failures (non-zero exitCode or JSON parse errors)
bypass the throttle and can repeatedly spawn execBv; update lastTriageRefreshAt
in all code paths that handle bv triage (both the exitCode !== 0 branch and the
catch block after JSON.parse) and ensure lastTriageOutput is set to a safe
default ({ triage: { recommendations: [] } }) on failures; apply the same change
to the other similar triage-handling block (the second execBv/parsing block) so
both paths always set lastTriageRefreshAt regardless of success or failure.
In `@website/content/docs/plugins/trackers/beads-rust-bv.mdx`:
- Around line 109-111: Update the documentation to state that the plugin option
labels accepts both a comma-separated string and an array by documenting its
type as "string | string[]"; update the inline config snippet (the block showing
epicId, labels, workingDir) and the options table entries referencing labels to
use "string | string[]" and add a short example or note explaining that a
comma-separated string and a string array are both supported so users
configuring via structured config know both formats work; target the labels
entry in this file (labels variable in the config snippet) and the options table
rows that describe labels.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/plugins/trackers/builtin/beads-rust-bv/index.tstests/plugins/beads-rust-bv-tracker.test.tswebsite/content/docs/plugins/trackers/beads-rust-bv.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/plugins/beads-rust-bv-tracker.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/trackers/builtin/beads-rust-bv/index.ts (1)
84-115: Consider adding a timeout to prevent indefinite blocking.If
bvhangs (e.g., due to a deadlock or waiting for input),execBvwill block indefinitely. This could freeze the TUI. Consider adding a timeout mechanism.💡 Proposed fix using AbortController
async function execBv( args: string[], - cwd?: string + cwd?: string, + timeoutMs = 30_000 ): Promise<{ stdout: string; stderr: string; exitCode: number }> { return new Promise((resolve) => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + const proc = spawn('bv', args, { cwd, env: { ...process.env }, stdio: ['ignore', 'pipe', 'pipe'], + signal: controller.signal, }); let stdout = ''; let stderr = ''; proc.stdout.on('data', (data: Buffer) => { stdout += data.toString(); }); proc.stderr.on('data', (data: Buffer) => { stderr += data.toString(); }); proc.on('close', (code) => { + clearTimeout(timeout); resolve({ stdout, stderr, exitCode: code ?? 1 }); }); proc.on('error', (err) => { + clearTimeout(timeout); stderr += err.message; resolve({ stdout, stderr, exitCode: 1 }); }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts` around lines 84 - 115, The execBv function can hang if the spawned 'bv' process stalls; add a timeout using an AbortController (or a timer) to terminate the child and resolve the promise when exceeded. Modify execBv to create an AbortController or setTimeout that calls proc.kill() (or proc.kill('SIGKILL')) and appends a timeout message to stderr, then resolve with stdout, stderr and a non-zero exitCode (e.g., 124) when the timer fires; ensure the timer is cleared on normal close/error and that the AbortController signal is passed to spawn if using the Abort API so the child is reliably cancelled.
🤖 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-rust-bv/index.ts`:
- Around line 84-115: The execBv function can hang if the spawned 'bv' process
stalls; add a timeout using an AbortController (or a timer) to terminate the
child and resolve the promise when exceeded. Modify execBv to create an
AbortController or setTimeout that calls proc.kill() (or proc.kill('SIGKILL'))
and appends a timeout message to stderr, then resolve with stdout, stderr and a
non-zero exitCode (e.g., 124) when the timer fires; ensure the timer is cleared
on normal close/error and that the AbortController signal is passed to spawn if
using the Abort API so the child is reliably cancelled.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/plugins/trackers/builtin/beads-rust-bv/index.tswebsite/content/docs/plugins/trackers/beads-rust-bv.mdx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Owner
Code reviewFound 4 issues:
ralph-tui/src/plugins/trackers/builtin/beads-rust-bv/index.ts Lines 269 to 345 in 786112b
ralph-tui/src/plugins/trackers/builtin/beads-rust-bv/index.ts Lines 321 to 340 in 786112b
ralph-tui/src/plugins/trackers/builtin/beads-rust-bv/index.ts Lines 474 to 512 in 786112b
Lines 162 to 168 in 786112b 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Add filter.status validation to getNextTask (regression vs beads-bv fix 35d2e12) - Call scheduleTriageRefresh in epic-filter path to prevent stale triage data - Move lastTriageRefreshAt update inside success branch to allow retries on failure - Wire task.metadata.bvReasons to selectionReason in engine buildPrompt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts`:
- Around line 282-289: The fallback to delegate.getNextTask(filter) can return a
task whose status is outside the requested statusFilter; update the calls (the
branches using statusFilter and calling this.delegate.getNextTask(filter)) to
validate the returned task's status against the original statusFilter and only
return it if it matches — otherwise continue searching or return null/undefined
as the contract requires. Concretely, after calling
this.delegate.getNextTask(filter) in the current function, check
returnedTask?.status and ensure statusFilter.some(s => s ===
returnedTask.status) (or the equivalent open/in_progress checks used elsewhere)
before returning; if it doesn't match, loop/try next or return no task. Ensure
the same fix is applied to the other similar branches noted (the delegate calls
around the same checks).
- Around line 501-529: The timestamp this.lastTriageRefreshAt is being updated
regardless of parse success; change the logic so the timestamp is only set when
parsing and validation succeed: inside the exitCode === 0 block, move the
this.lastTriageRefreshAt = Date.now() into the branch where parsed is validated
and this.lastTriageOutput is set to the parsed value (the block that filters
recommendations and assigns this.lastTriageOutput), and do not update the
timestamp in the catch or the "invalid parsed" else that sets an empty triage;
reference symbols: exitCode, stdout, BvTriageOutput, BvTriageRecommendation,
this.lastTriageOutput, this.lastTriageRefreshAt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22614337-b5c9-4a30-8615-4bb371ae10c3
📒 Files selected for processing (2)
src/engine/index.tssrc/plugins/trackers/builtin/beads-rust-bv/index.ts
Owner
|
Pushed a fix commit ( 1. 2. 3. 4. |
- Wrap all delegate.getNextTask fallbacks with status validation via delegateGetNextTaskFiltered() so delegate can't return tasks outside the caller's statusFilter - Move lastTriageRefreshAt update into the successful parse+validate branch only, so parse failures and invalid structures also allow immediate retries
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/trackers/builtin/beads-rust-bv/index.ts`:
- Around line 84-115: The execBv function currently waits only for
'close'/'error' and can hang if the spawned bv process stalls; add a 15-second
timeout using setTimeout when the process is spawned, call proc.kill('SIGKILL')
when the timer fires, append a timeout message to stderr and resolve the promise
with a non-zero exitCode, and ensure the timeout is cleared in both the
proc.on('close') and proc.on('error') handlers to avoid leaks; update references
in execBv to manage the timer variable and ensure consistent resolution path
when killed by timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 157f1247-594a-4fba-8d6f-8ebc4ca60b6f
📒 Files selected for processing (1)
src/plugins/trackers/builtin/beads-rust-bv/index.ts
added 4 commits
March 13, 2026 11:20
The installBuiltinTemplates test expected 4 templates but beads-rust-bv was added to the map without updating the assertion.
If bv stalls (deadlock, waiting for input), execBv now kills the process after 15 seconds and resolves with a non-zero exit code instead of hanging indefinitely.
beads-rust-bv-tracker.test.ts mocks node:child_process and node:fs/promises via mock.module() which is process-wide in Bun. Running it alongside other tests causes 27 failures in json-tracker, gemini-agent detect, and Config Push tests. Matches the existing isolation pattern used for beads-bv and beads-rust tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 47.39% 47.63% +0.24%
==========================================
Files 103 104 +1
Lines 33404 33923 +519
==========================================
+ Hits 15831 16160 +329
- Misses 17573 17763 +190
🚀 New features to boost your workflow:
|
Owner
|
Also pushed a few more commits to get CI green:
|
Owner
|
Thanks for the contribution @cachemoney 🙌 |
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 tracker supports beads-rust with using bv as the source to ralph-tui pick up next beads to work on. The beads-rust tracker only supports beads, bd, cli.
Summary by CodeRabbit
New Features
Templates
Documentation
Tests
UI