src/plugins/trackers/builtin/beads-rust/index.ts (2)

452-461: enrichDependencies is inserted between getNextTask's JSDoc and its implementation.

The JSDoc block at lines 445–451 documents getNextTask (which is defined at line 537). Inserting enrichDependencies here creates an 86-line gap between that comment and the method it annotates, making the JSDoc appear orphaned and the class structure confusing to navigate.

Move enrichDependencies immediately before or after getChildIds (line 511) so that getNextTask's JSDoc remains directly adjacent to getNextTask.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 452 - 461, The
JSDoc for getNextTask was orphaned by inserting enrichDependencies in the
middle; move the enrichDependencies function so the getNextTask comment sits
immediately above getNextTask again—specifically cut the enrichDependencies
implementation and paste it immediately before or immediately after getChildIds
so getNextTask's JSDoc and the getNextTask method (function getNextTask) are
adjacent.

---

503-503: Unbounded Promise.all may exhaust process/FD limits for large epics.

All br dep list spawns are issued concurrently with no cap. For an epic with a large number of tasks whose dependency_count > 0, this fans out to that many simultaneous child processes. Consider chunking or using a concurrency limiter (e.g., p-limit) to keep the fan-out bounded.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` at line 503, The unbounded
await Promise.all(enrichPromises) fans out all 'br dep list' child processes at
once; change this to a bounded concurrency approach by replacing the direct
Promise.all(enrichPromises) with a concurrency-limited runner (e.g., use p-limit
or a simple chunked loop). Create a limiter (const limit = pLimit(CONCURRENCY))
and build limitedPromises = enrichPromises.map(p => limit(() => p)) then await
Promise.all(limitedPromises), or iterate over enrichPromises in fixed-size
chunks and await each chunk before proceeding; reference the enrichPromises
array and the place where Promise.all(enrichPromises) is awaited to apply the
change and pick a sensible CONCURRENCY (e.g., 5-20).

src/plugins/trackers/builtin/beads-rust/index.ts (2)

495-500: depCountMap stores count values that are never read — use Set<string> instead.

The Map<string, number> stores dependency_count values (line 498) but the only usage is depCountMap.has(t.id) (line 504); the numeric value is never accessed. A Set<string> is semantically cleaner and avoids the misleading implication that counts are consumed.

♻️ Proposed refactor

-    const depCountMap = new Map<string, number>();
+    const tasksWithDeps = new Set<string>();
     for (const raw of rawTasks) {
       if (typeof raw.dependency_count === 'number' && raw.dependency_count > 0) {
-        depCountMap.set(raw.id, raw.dependency_count);
+        tasksWithDeps.add(raw.id);
       }
     }

     const enrichPromises = tasks
-      .filter((t) => depCountMap.has(t.id))
+      .filter((t) => tasksWithDeps.has(t.id))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 495 - 500, The
code currently creates depCountMap as a Map<string, number> but never reads the
numeric values; change it to a Set<string> named e.g. depWithDeps and populate
it from rawTasks by checking raw.dependency_count > 0 and calling
depWithDeps.add(raw.id) (replace depCountMap.set(...)). Update downstream checks
that call depCountMap.has(t.id) to use depWithDeps.has(t.id). Ensure the symbol
depWithDeps (or a renamed Set) replaces depCountMap everywhere in this module.

---

383-387: enrichDependencies fires before parent filtering, causing unnecessary br dep list calls for tasks that will be discarded.

enrichDependencies is called at line 387 on the full, unfiltered tasks array. The parent/child filter is applied at lines 391–394 via getChildIds, which may discard the majority of tasks. Every task with dependency_count > 0 in the entire tracker (not just the epic's children) gets its own br dep list subprocess call, even though those results are thrown away.

Reorder so that parent filtering happens first, then enrich only the surviving tasks:

♻️ Proposed reordering

     let tasks = tasksJson.map(brTaskToTask);

-    // Enrich tasks with dependency data from br dep list.
-    await this.enrichDependencies(tasks, tasksJson);
-
     // Filter by parent (br list doesn't support --parent)
     if (parentId) {
       const childIds = await this.getChildIds(parentId);
       tasks = tasks.filter((t) => childIds.has(t.id));
     }

+    // Enrich surviving tasks with dependency data from br dep list.
+    await this.enrichDependencies(tasks, tasksJson);
+
     // Apply remaining filters (excluding parentId which we handled above)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 383 - 387, The
call to enrichDependencies is happening on the full tasks array so br dep list
is invoked for tasks that will be filtered out later; change the order so you
first compute the parent/child filtered set using getChildIds (apply the same
filtering logic that produces the subset of tasks/children) and then call
enrichDependencies only with that filtered tasks array (and the corresponding
filtered tasksJson) so subprocess calls are only made for surviving tasks;
ensure any later code that expects enrichment uses the filteredTasks variable
(update references that previously assumed the original tasks array).

src/plugins/trackers/builtin/beads-rust/index.test.ts (2)

509-543: Consider adding a lower-bound assertion on maxEnrichmentDepCalls.

The test validates the upper bound (<= 8) but would still pass if the implementation regressed to serial execution. A complementary toBeGreaterThan(1) assertion would guard against accidentally losing parallelism.

💡 Proposed addition

       expect(depEnrichmentCalls.length).toBe(taskCount);
       expect(maxEnrichmentDepCalls).toBeLessThanOrEqual(8);
+      expect(maxEnrichmentDepCalls).toBeGreaterThan(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/index.test.ts` around lines 509 -
543, Add a lower-bound assertion to the test 'limits concurrent dependency
enrichment calls to avoid process fan-out' to ensure parallelism isn't lost:
after the existing upper-bound check on maxEnrichmentDepCalls, add an assertion
that maxEnrichmentDepCalls is greater than 1 (e.g.,
expect(maxEnrichmentDepCalls).toBeGreaterThan(1)). This should be added in the
same test that constructs BeadsRustTrackerPlugin, calls plugin.getTasks(), and
inspects depEnrichmentCalls to validate concurrency behavior.

---

468-543: Consider adding a test for enrichment failure resilience.

The implementation silently ignores br dep list failures during enrichment (per the code summary). A test that returns a non-zero exit code or invalid JSON for one task's enrichment call — while succeeding for another — would confirm that partial enrichment failures don't break the overall getTasks flow and that unresolvable tasks simply have no dependsOn.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.test.ts` around lines 468 -
543, Add a test to verify enrichment failure resilience: in
src/plugins/trackers/builtin/beads-rust/index.test.ts create a new test that
uses BeadsRustTrackerPlugin and getTasks with multiple tasks where
mockSpawnResponses includes a successful list response for some tasks and a
failing enrichment response for at least one task (set exitCode != 0 or invalid
JSON for that task's 'br dep list' entry). After calling plugin.getTasks(),
assert that all tasks are returned (tasks.length), that the task with the
failing enrichment has no dependsOn (undefined or empty), and that another task
with a successful enrichment has its dependsOn populated; also assert
mockSpawnArgs contains the expected 'dep list' calls to ensure enrichment was
attempted. Ensure you reset/prepare mockSpawnArgs and toggle
trackEnrichmentDepConcurrency as needed to reproduce concurrent behavior.

src/plugins/trackers/builtin/beads-rust/index.ts (1)

481-506: Optional: rawTasks parameter can be dropped — the data is already on task.metadata.

brTaskToTask already writes dependency_count into task.metadata.dependencyCount (lines 216–218). The depCountMap built here from rawTasks replicates that information. enrichDependencies could filter tasks directly and remove the second parameter entirely:

♻️ Proposed simplification

-  private async enrichDependencies(
-    tasks: TrackerTask[],
-    rawTasks: BrTaskJson[]
-  ): Promise<void> {
-    // Build a map of task ID → raw dependency_count for quick lookup
-    const depCountMap = new Map<string, number>();
-    for (const raw of rawTasks) {
-      if (typeof raw.dependency_count === 'number' && raw.dependency_count > 0) {
-        depCountMap.set(raw.id, raw.dependency_count);
-      }
-    }
-
-    const tasksToEnrich = tasks.filter((t) => depCountMap.has(t.id));
+  private async enrichDependencies(tasks: TrackerTask[]): Promise<void> {
+    const tasksToEnrich = tasks.filter(
+      (t) => typeof t.metadata?.dependencyCount === 'number' && (t.metadata.dependencyCount as number) > 0
+    );
     if (tasksToEnrich.length === 0) {
       return;
     }

And the call site:

-    await this.enrichDependencies(tasks, tasksJson);
+    await this.enrichDependencies(tasks);

This is a style preference — the current approach is also correct and slightly more type-safe.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 481 - 506,
enrichDependencies currently takes rawTasks and builds depCountMap from
raw.dependency_count but brTaskToTask already populates the same value into
task.metadata.dependencyCount; remove the rawTasks parameter from
enrichDependencies, change its signature to enrichDependencies(tasks:
TrackerTask[]), update the filtering to use tasks.filter(t => typeof
t.metadata?.dependencyCount === 'number' && t.metadata.dependencyCount > 0') (or
similar), and adjust the caller(s) to stop passing rawTasks; keep the batching
logic and the call to enrichTaskDependencies(task) unchanged.

src/plugins/trackers/builtin/beads-rust/index.test.ts (2)

67-87: The enrichment-call detection heuristic is implicitly coupled to the absence of --direction.

isEnrichmentDepList identifies enrichment calls by the absence of --direction rather than positively asserting an expected arg set. If enrichTaskDependencies ever adds a direction flag, this test silently stops tracking what it claims to track — maxEnrichmentDepCalls would never increment and the concurrency cap assertion would trivially pass.

Consider adding a brief comment to surface this coupling, or matching more precisely:

♻️ Proposed tighter detection

-        const isEnrichmentDepList =
-          args[0] === 'dep' &&
-          args[1] === 'list' &&
-          !args.includes('--direction');
+        // Enrichment calls: ['dep', 'list', <taskId>, '--json']
+        // Parent-filter calls include '--direction up'; exclude those.
+        const isEnrichmentDepList =
+          args[0] === 'dep' &&
+          args[1] === 'list' &&
+          args[args.length - 1] === '--json' &&
+          !args.includes('--direction');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.test.ts` around lines 67 - 87,
The test's enrichment-call detection (isEnrichmentDepList) relies on negating
'--direction' which couples it to enrichTaskDependencies' implementation; update
the heuristic to match the exact expected arg pattern instead (e.g., assert args
include 'dep' and 'list' and include any other expected flags/values) or add an
explicit comment documenting the coupling so future changes don't silently break
tracking; edit the detection in the mock spawn stub where isEnrichmentDepList is
defined (and adjust the close handler logic that decrements
activeEnrichmentDepCalls) to either perform a positive match of the exact arg
set or add the clarifying comment and tests to ensure the '--direction' change
fails loudly.

---

575-609: Concurrency cap test only validates the upper bound; a regression to serial execution would go undetected.

The assertion expect(maxEnrichmentDepCalls).toBeLessThanOrEqual(8) would trivially pass if the batch size regressed to 1 (fully serial). Adding a lower-bound assertion (e.g. toBeGreaterThan(1) or toBeGreaterThanOrEqual(8) for the first full batch of 18 tasks split into two full batches of 8) would make the test a genuine two-sided guard.

  expect(maxEnrichmentDepCalls).toBeLessThanOrEqual(8);
+ // Confirm the first batch is actually processed concurrently (not serial)
+ expect(maxEnrichmentDepCalls).toBeGreaterThan(1);

Separately, there is no test for the enrichment failure path (exitCode !== 0 or invalid JSON from br dep list). Adding a test that supplies a failing response for the enrichment call and asserts task.dependsOn is still defined (from the BrTaskJson.dependencies field) would cover the silent-fallback behaviour discussed in the implementation review.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.test.ts` around lines 575 -
609, Update the concurrency-cap test to assert both an upper and a lower bound
so serial regression is caught: in the test that constructs
BeadsRustTrackerPlugin, calls plugin.initialize and plugin.getTasks and inspects
maxEnrichmentDepCalls, add a lower-bound expectation (e.g.
expect(maxEnrichmentDepCalls).toBeGreaterThan(1) or
expect(...).toBeGreaterThanOrEqual(8) depending on intended batch semantics) in
addition to the existing toBeLessThanOrEqual(8). Also add a new test that
injects a failing enrichment response into mockSpawnResponses (simulate exitCode
!== 0 or invalid JSON for the 'br dep list' calls) and assert that tasks
returned by plugin.getTasks still have task.dependsOn populated from the
original BrTaskJson.dependencies field (verify fallback behavior), referencing
the existing test setup pieces like mockSpawnResponses, mockSpawnArgs,
BeadsRustTrackerPlugin, plugin.initialize and plugin.getTasks to locate where to
add these checks.

src/plugins/trackers/builtin/beads-rust/index.ts (2)

486-491: depCountMap stores values that are never read — prefer Set<string>.

The count stored in the Map is accessed nowhere; only .has() is used on line 493. A Set<string> is both cleaner in intent and avoids the spurious numeric storage.

♻️ Proposed refactor: Map → Set

-    const depCountMap = new Map<string, number>();
-    for (const raw of rawTasks) {
-      if (typeof raw.dependency_count === 'number' && raw.dependency_count > 0) {
-        depCountMap.set(raw.id, raw.dependency_count);
-      }
-    }
-
-    const tasksToEnrich = tasks.filter((t) => depCountMap.has(t.id));
+    const depsTaskIds = new Set<string>();
+    for (const raw of rawTasks) {
+      if (typeof raw.dependency_count === 'number' && raw.dependency_count > 0) {
+        depsTaskIds.add(raw.id);
+      }
+    }
+
+    const tasksToEnrich = tasks.filter((t) => depsTaskIds.has(t.id));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 486 - 491,
depCountMap is a Map<string, number> that only uses .has() later, so replace it
with a Set<string> to reflect intent and avoid storing unused numbers: when
iterating rawTasks check typeof raw.dependency_count === 'number' &&
raw.dependency_count > 0 and add raw.id to a new Set (e.g., depIdSet) instead of
depCountMap.set(...), then update subsequent checks that call
depCountMap.has(...) to use depIdSet.has(...); ensure symbols referenced are
depCountMap -> depIdSet, rawTasks, raw.dependency_count, and raw.id.

---

498-505: Consider semaphore-based concurrency, but p-limit is not a direct dependency and would need to be added first.

The batch-slicing approach does stall when a single br dep list call is slow. A semaphore pattern (e.g., p-limit) maintains a rolling window, starting the next call as soon as any slot frees. However, p-limit is not currently a direct dependency; it exists only as a transitive dependency. Using it without explicit declaration is unreliable. Either add p-limit to package.json dependencies, or consider a native alternative using a queue-based approach with Promise.allSettled() or a custom semaphore implementation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/trackers/builtin/beads-rust/index.ts` around lines 498 - 505,
Replace the batch-slicing concurrency in the loop with a semaphore-based
limiter: add p-limit to package.json and use it to enforce
maxConcurrentEnrichCalls, creating a limiter via
pLimit(maxConcurrentEnrichCalls) and mapping tasksToEnrich to limiter(() =>
this.enrichTaskDependencies(task)) then await Promise.all on those promises;
target the loop and the this.enrichTaskDependencies calls so the limiter issues
a new call as soon as any slot frees. Alternatively, if adding a dependency is
undesired, implement a tiny in-file semaphore/queue that maintains a rolling
window, dequeues the next task when a slot frees, and invokes
this.enrichTaskDependencies(task) until tasksToEnrich is exhausted.