Conversation
Complexity Assessment for Issue #557Analysis Plan
Complexity AssessmentClassification: COMPLEX Metrics:
Reasoning: This epic implements a sophisticated multi-agent orchestration system with external Beads integration, requiring coordination across 14+ files, significant new code (~1000 LOC), and critical architectural decisions about supervisor loops, conflict resolution agents, and parallel task claiming. The cross-cutting nature of the data flow (epic detection threading through supervisor → task claiming → worktree → agent → merge) and multiple integration points with uncertain implementation approaches trigger automatic COMPLEX classification despite moderate individual file impacts. |
6f1f1e6 to
f9ecc92
Compare
Code Review: Epic #557 — Autonomous Swarm ModeReview of all 7 commits (#558-#564) against their respective issue requirements, ADR compliance, and project guidelines. Critical / High Priority (Must Fix)1. Silent auto-install in CI when
|
| # | Commit | Location | Issue |
|---|---|---|---|
| 20 | #558 | BeadsManager.ts (runInstallScript) |
curl|bash from unpinned main branch — no integrity verification |
| 21 | #558 | BeadsManager.ts (execBd) |
Spreads full process.env to bd subprocess — potential secret leakage |
| 22 | #558 | BeadsSyncService.test.ts |
Mock missing providerName and issuePrefix properties |
| 23 | #558 | plan-prompt.txt:280-283 |
Duplicate numbered list items (two 1. entries) |
| 24 | #559 | start.ts:722 |
Settings loaded twice (in execute and detectEpic) |
| 25 | #559 | start.ts:225-228 |
--swarm on null epicDetection produces no log message |
| 26 | #560 | LoomManager.ts:~1186 |
Missing extractIssueNumber for PR-type swarm looms |
| 27 | #562 | SwarmSupervisor.ts:372-376 |
Force shutdown orphans child processes |
| 28 | #562 | SwarmSupervisor.ts:279-289 |
Graceful shutdown has no timeout — can hang indefinitely |
| 29 | #562 | SwarmSupervisor.ts:304-310 |
PR search by issueId in:title can match wrong PRs |
| 30 | #563 | BeadsManager.ts:256-264 |
list() silently returns [] on parse failure (same as #13) |
| 31 | #563 | SwarmSupervisor.ts |
maxRetries=1 means 1 total attempt, 0 retries — naming misleading |
| 32 | #563 | SwarmSupervisor.ts:844 |
Progress file not written atomically — readers could see partial JSON |
| 33 | #564 | start-swarm.test.ts:303 |
Dynamic import in test violates project guidelines |
| 34 | #564 | start.ts:763 |
No range validation for --max-agents CLI flag |
Positive Observations
- Clean dependency injection throughout — all core classes accept dependencies via constructor
- Good error typing with
BeadsErrorpreserving exit code and stderr - Well-structured test coverage: 98 new tests across 7 commits
- Clean separation of concerns: BeadsManager (CLI), BeadsSyncService (sync), EpicDetector (detection), SwarmSupervisor (orchestration)
- Swarm fast path cleanly short-circuits without touching normal flow
- Signal handler install/removal in try/finally prevents handler leaks
- Template conditionals preserve non-swarm workflow unchanged
Code Review Fixes AppliedAll 34 issues from the code review have been addressed in commit Critical/High (7 fixes)
Medium (12 fixes)
Low (15 fixes)Issues #20-34: Security comments for unpinned install, env filtering for subprocess, mock property fixes, duplicate list numbering, Validation
|
Implementation Complete - Beads ID format fixSummaryFixed bug where Beads CLI rejected task IDs because they were plain GitHub issue numbers (e.g., Changes Made
Validation Results
Detailed Changes by File (click to expand)src/lib/BeadsSyncService.tsChanges: Added
src/lib/SwarmSupervisor.tsChanges: Fixed ID mapping for GitHub API calls that need raw issue numbers
src/lib/BeadsSyncService.test.tsChanges: Updated all test expectations and added helper function tests
src/lib/SwarmSupervisor.test.tsChanges: Updated all Beads task IDs and assertions to use
|
Analysis: Research
|
| Flag | Description |
|---|---|
--prefix/-p |
Issue prefix |
--quiet/-q |
Suppress output |
--branch/-b |
Git branch for beads commits |
--backend |
Storage backend (sqlite/dolt) |
--force |
Force re-initialization |
--contributor |
Run contributor wizard |
--team |
Run team wizard |
--stealth |
Enable stealth mode |
--setup-exclude |
Configure git exclude for forks |
--skip-hooks |
Skip git hooks installation |
--skip-merge-driver |
Skip merge driver setup |
--from-jsonl |
Import from existing issues.jsonl |
No --role flag exists.
Analysis: Beads CLI (
|
| Question | Answer |
|---|---|
Should the prefix be a short token like gh or the full repo slug like iloom-test-project-github? |
Using the full repo slug prevents collisions across repos but creates verbose IDs. Beads supports any prefix format; this is an iloom design choice. |
Should we use --prefix on init, or --force on create? |
--prefix on init is the clean approach. --force bypasses all validation and is a blunt workaround. |
| Where should the prefix be derived from (repo name, org/repo, custom)? | The prefix needs to be consistent between init and ID generation. It can be derived from the repo slug or be configurable via swarm settings. |
Impact Summary
- 2 files requiring modification:
BeadsManager.ts(init method) andBeadsSyncService.ts(toBeadsId/fromBeadsId) - The prefix choice must be consistent between init and create calls
- Existing tests in
BeadsSyncService.test.tsassumegh-prefix and will need updating if the prefix changes
Complete Technical Reference (click to expand for implementation details)
Problem Space Research
Problem Understanding
When iloom syncs GitHub child issues into Beads for swarm mode DAG orchestration, bd create --id gh-54 fails because the Beads database was initialized with prefix iloom-test-project-github (auto-detected from the directory name), but the ID gh-54 starts with gh, not iloom-test-project-github.
Architectural Context
Beads enforces prefix consistency: all IDs in a database must share the same prefix. This is by design to prevent cross-project contamination when databases are shared. iloom stores Beads databases outside the repo at ~/.config/iloom-ai/beads/<project-hash>, so cross-project contamination is already prevented by the project-hash directory isolation.
Edge Cases Identified
- Multiple repos with same name: Two repos named
my-appin different orgs would hash to different BEADS_DIR paths (iloom uses project path hash), so prefix collision is not a concern at the filesystem level - Prefix with special characters: Beads normalizes prefixes by stripping trailing hyphens. The prefix
iloom-test-project-githubis valid but verbose - Linear integration: Linear IDs like
ENG-123already have a prefix format. If Linear is the issue tracker, the prefix should match (e.g.,ENG)
Third-Party Research Findings
Beads CLI (Go, ~4 months old)
Source: GitHub source code at https://github.com/steveyegge/beads (cloned and analyzed directly)
How bd init Sets the Prefix
Prefix determination follows strict precedence (from cmd/bd/init.go:122-152):
--prefix/-pflag: Highest priority.bd init --prefix ghsets prefix togh- config.yaml value:
config.GetString("issue-prefix")from.beads/config.yaml - Auto-detect from git history: Scans JSONL for existing issues, extracts prefix from first issue
- Directory name fallback:
filepath.Base(cwd)- the basename of CWD
After determination, trailing hyphens are stripped: strings.TrimRight(prefix, "-").
The prefix is permanently stored in SQLite: store.SetConfig(ctx, "issue_prefix", prefix) at init.go:369.
How bd create --id Validates
From cmd/bd/create.go:458-483:
- Reads
issue_prefixfrom database (or daemon RPC) - Also reads
allowed_prefixesfor multi-prefix support - Calls
validation.ValidateIDPrefixAllowed(explicitID, dbPrefix, allowedPrefixes, forceCreate)
The validation (internal/validation/bead.go:139-167) checks:
- If
forceis true, skip all validation - If
dbPrefixis empty, skip validation - If
idstarts withdbPrefix + "-", pass - If
idstarts with any entry inallowedPrefixes + "-", pass - Otherwise: error with
"prefix mismatch: database uses '{dbPrefix}-' but ID '{id}' doesn't match (use --force to override)"
ID Format Requirements
From internal/validation/bead.go:55-76 (ValidateIDFormat):
- Must contain at least one hyphen
- Format:
prefix-hashorprefix-number(e.g.,bd-a3f8e9,bd-42) - Hierarchical:
prefix-hash.number(e.g.,bd-a3f8.1) - Multi-hyphen prefixes supported:
web-app-a3f8e9extracts as prefixweb-app
Querying the Current Prefix
Two methods:
bd info --jsonreturns{ "issue_prefix": "..." }among other fieldsbd config get issue_prefixreads from database (not documented for this key but uses same mechanism)
Multi-Prefix Support
allowed_prefixes is a comma-separated config value stored in the database: store.GetConfig(ctx, "allowed_prefixes"). IDs matching any prefix in this list (plus the primary issue_prefix) pass validation.
--force Bypass
bd create --id gh-54 --force skips all prefix validation. This is the emergency escape hatch but is not the intended workflow.
Codebase Research Findings
Affected Area: BeadsManager + BeadsSyncService
Entry Point for the bug: BeadsManager.init() at /Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsManager.ts:168-181
The init method calls bd init --quiet --skip-hooks --skip-merge-driver without --prefix. Since BEADS_DIR is set to a directory outside the repo, and cwd is set to projectPath, Beads auto-detects the prefix from filepath.Base(projectPath).
For repo iloom-ai/iloom-test-project-github, if the checkout is at a path ending in iloom-test-project-github, the prefix becomes iloom-test-project-github.
ID Generation: BeadsSyncService.toBeadsId() at /Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsSyncService.ts:30-32
This function prefixes numeric GitHub IDs with gh- (e.g., 54 becomes gh-54). This hardcoded gh- prefix does not match the database prefix.
Dependencies:
BeadsManager.init()is called fromSwarmSupervisorduring epic startupBeadsSyncService.syncEpicToBeads()callsBeadsManager.create()with IDs fromtoBeadsId()toBeadsId()andfromBeadsId()are used throughout swarm code for ID conversion
Key Design Tension
The user's stated preference is IDs that include repo info to prevent collisions (e.g., iloom-test-project-github-54). However, iloom already isolates Beads databases per-project via a SHA-256 hash of the project path (BeadsManager.computeProjectHash()), so cross-project collision is already impossible at the filesystem level. The question is whether human-readable disambiguation is worth the verbosity.
Possible Approaches (for reference, not recommendation)
- Pass
--prefixtobd initwith a value that matches whattoBeadsId()generates (e.g.,gh) - Change
toBeadsId()to generate IDs that match whatever prefix Beads auto-detects - Use
--forceon everybd createcall to bypass validation - Use
allowed_prefixesto registerghas an additional allowed prefix after init
Affected Files
/Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsManager.ts:168-181-init()method does not pass--prefixtobd init/Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsSyncService.ts:30-32-toBeadsId()hardcodesgh-prefix/Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsSyncService.ts:40-42-fromBeadsId()hardcodesgh-stripping/Users/adam/Documents/Projects/iloom-cli/feat-issue-557__autonomous-swarm-mode/src/lib/BeadsSyncService.test.ts- Tests assumegh-prefix format throughout
Integration Points
BeadsManager.init()is called bySwarmSupervisorwhich passesthis.projectPathas CWDBeadsSyncServiceusesBeadsManager.create()with IDs fromtoBeadsId()fromBeadsId()is used to convert Beads task IDs back to issue tracker IDs for GitHub API calls- The
bdCLI readsBEADS_DIRenv var set byBeadsManager.execBd()for all operations
Beads CLI Reference Summary
| Command | Relevant Behavior |
|---|---|
bd init --prefix <value> |
Sets prefix explicitly, stored in SQLite |
bd init (no --prefix) |
Auto-detects: config.yaml > git history > directory name |
bd create --id <id> |
Validates ID starts with {prefix}- |
bd create --id <id> --force |
Bypasses prefix validation |
bd info --json |
Returns issue_prefix among other config |
bd config set allowed_prefixes "gh,other" |
Adds allowed prefixes for multi-prefix support |
Implementation Complete - Fix Beads task ID prefix systemSummaryReplaced the hardcoded Changes Made
Validation Results
Detailed Changes by File (click to expand)Files Modified
|
Implementation: Simplify Beads prefix system
|
Implementation Complete - Issue #557 (Simplify Beads Prefix) ✅SummaryRemoved stale Changes Made
Validation Results
📋 Detailed Changes by File (click to expand)Files Modified
|
Implementation Complete - Add user-visible logging to Beads DAG syncSummaryAdded user-visible Changes Made
Validation Results
Detailed Changes by File (click to expand)Files Modifiedsrc/lib/BeadsSyncService.tsChanges: Upgraded 3 debug log calls to info-level user-visible output, added 1 new dependency log line
src/lib/SwarmSupervisor.tsChanges: Removed redundant summary line after sync (line 215 previously: src/lib/BeadsSyncService.test.tsChanges: Added
|
Implementation: Fix swarm mode worktree reuse bug
|
Implementation Complete - Issue #557 (Swarm Mode Reuse Path Fix)SummaryFixed the bug where Changes Made
Validation Results
Detailed Changes by File (click to expand)Files Modified
|
Analysis: SwarmSupervisor Bugs (Issue #557)
Executive SummaryTwo bugs in HIGH/CRITICAL Risks
Impact Summary
Complete Technical Reference (click to expand for implementation details)Problem Space ResearchProblem UnderstandingThe swarm supervisor orchestrates N child agents working on an epic's sub-issues. It relies on Beads (a DAG task tracker) to determine which tasks are ready (unblocked). When a task completes, closing it in Beads should unblock dependent tasks, making them appear in subsequent Architectural ContextThe supervisor loop follows a poll-based model: each iteration queries Codebase Research FindingsBug 1: Premature Loop ExitRoot Cause: The exit condition at const actionableReadyTasks = readyTasks.filter(t => !this.permanentlyFailed.has(t.id))
if (actionableReadyTasks.length === 0 && this.activeAgents.size === 0 && this.mergeQueue.length === 0 && this.pendingReleases === 0) {
break
}This checks only the current
Contributing factor: The correct exit condition should verify that Bug 2: Rate-Limited PR SearchEntry Point: Mechanism:
Sub-issues with the search query:
Affected Files
Similar Patterns Found
Edge Cases Identified
Medium Severity Risks
|
Combined Analysis & Plan - Issue #557 (Swarm Bugs)Executive SummaryThree bugs in HIGH/CRITICAL Risks
Implementation OverviewHigh-Level Execution Phases
Quick Stats
Complete Analysis & Implementation Details (click to expand)Research FindingsProblem Space
Codebase Research
Affected Files
Medium Severity Risks
Implementation PlanAutomated Test Cases to CreateTest File: Click to expand complete test structure (35 lines)describe('premature exit fix', () => {
it('should NOT exit when bd ready returns empty but tasks remain incomplete', async () => {
// Setup: 3 tasks synced, task 1 ready, tasks 2+3 blocked on task 1
// Cycle 1: bd ready returns [task1], claim+spawn, agent completes
// Cycle 2: bd ready returns [] (task1 just closed, task2/3 not yet unblocked)
// Verify: loop does NOT exit. Cycle 3: bd ready returns [task2, task3]
// result.completed should be 3, not 1
})
})
describe('closeTask error propagation', () => {
it('should propagate closeTask error in processMergeQueue and count as failure', async () => {
// Setup: agent succeeds, PR found, merge succeeds, bd close throws
// Verify: error is thrown/caught at processMergeQueue level, task counted as failed
})
it('should still log-and-continue for closeTask errors in handleAgentFailure', async () => {
// Setup: agent fails, exhausts retries, bd close throws
// Verify: still counted as failed, no crash
})
})
describe('findPRForBranch uses --head', () => {
it('should search PR by head branch name instead of title search', async () => {
// Verify executeGhCommand called with ['pr', 'list', '--state', 'open', '--json', 'number,headRefName', '--head', '<branch-name>']
})
})
describe('rate limit retry', () => {
it('should retry on rate limit error with backoff', async () => {
// Setup: executeGhCommand fails with rate limit error, then succeeds
// Verify: retried and succeeded
})
it('should give up after max retries', async () => {
// Setup: executeGhCommand keeps failing with rate limit
// Verify: eventually throws
})
})Test File: describe('executeGhCommandWithRetry', () => {
// Test retry on rate limit (403 + "rate limit")
// Test no retry on non-rate-limit errors
// Test max retries respected
})Files to Modify1.
|
Summary: Swarm Bug Fixes & Resilience ImprovementsThree critical swarm mode bugs were fixed plus resilience improvements: Bug 1: Premature Loop Exit (Critical)Problem: The swarm orchestration loop exited after completing just 1 of 6 tasks. The exit condition only checked if Fix: Added Bug 2: Imprecise PR SearchProblem: Fix:
Bug 3: No Rate Limit ResilienceProblem: When GitHub API calls hit rate limits, the supervisor just failed and moved on. Critical operations like PR merge and PR search had no retry logic. Fix: Added Bug 4: closeTask Error SwallowingProblem: Fix: Made Additional Fix: Progress Logging
OOM Investigation FindingDuring testing, discovered that Fix: Added Lesson learned: Mocking Node.js built-in modules like Files Changed
|
…a, and epic label support (#558)
…and sequential merge queue (#562)
…and progress reporting (#563)
…ntation Fix all critical, medium, and low priority issues from code review: - Eliminate error swallowing in BeadsManager, SwarmSupervisor, and start.ts - Add input validation for --max-agents (NaN check + range 1-20) - Fix silent auto-install in CI when autoInstallBeads is false - Update PATH after Beads install for verification check - Prevent infinite loop on permanently failed tasks - Fix conflict resolver using wrong working directory - Close log file streams on agent completion - Add 30s timeout to graceful shutdown - Write progress file atomically via temp+rename - Fix broken prompt interpolation in confirmSwarmMode - Set GIT_REMOTE and validate EPIC_BRANCH in swarm mode - Make templates swarm-aware (autonomous mode, SKIP_IMPLEMENTATION) - Parallel dependency fetching via Promise.allSettled - Filter subprocess environment to prevent secret leakage - Fix parseInt truncation for mixed-format task IDs - Propagate isEpic/swarmStatus metadata in swarm finish path - Replace dynamic import with static import in tests
7991e33 to
e4d4a83
Compare
Catch "already initialized" error from `bd init` and treat as success, fixing re-runs on existing epic looms. Also improve execBd() error handling to distinguish CLI failures from unexpected runtime errors. Fixes #571
PR for issue #557
This PR was created automatically by iloom.