Skip to content

Proj/swarm memory#372

Merged
leehpham merged 39 commits intomainfrom
proj/swarm-memory
Apr 14, 2026
Merged

Proj/swarm memory#372
leehpham merged 39 commits intomainfrom
proj/swarm-memory

Conversation

@ngduyanhece
Copy link
Copy Markdown
Contributor

Summary

  • Problem: Knowledge is siloed across tools — ByteRover context-tree, Obsidian vaults, GBrain, and local markdown folders don't talk to each other. Users must manually decide where to search and where to store knowledge.
  • Why it matters: Developers waste time context-switching between knowledge tools. The AI agent can only search ByteRover's context tree, missing relevant knowledge in other sources.
  • What changed: Added a federated Memory Swarm system that connects all knowledge providers into a single search and write interface. Includes 4 CLI commands, 2 agent tools, sandbox integration, DAG-based enrichment topology, RRF result fusion, write routing, and a 5-tier search precision pipeline. 68 files, ~10K lines, 288 unit tests.
  • What did NOT change (scope boundary): ByteRover's existing context-tree search (brv curate, search_knowledge tool) is untouched. Honcho and Hindsight adapters are not implemented (temporarily disabled). GBrain precision filtering is out of scope (handled by its own hybrid search). No changes to TUI rendering, LLM providers, or daemon transport.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes #
  • Related #

Root cause (bug fixes only, otherwise write N/A)

N/A

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s):
    • test/unit/agent/swarm/search-precision.test.ts (18 tests)
    • test/unit/agent/swarm/adapters/byterover-adapter.test.ts (14 tests)
    • test/unit/agent/swarm/adapters/obsidian-adapter.test.ts (17 tests)
    • test/unit/agent/swarm/adapters/local-markdown-adapter.test.ts (19 tests)
    • test/unit/agent/swarm/adapters/gbrain-adapter.test.ts (14 tests)
    • test/unit/agent/swarm/swarm-coordinator.test.ts (12 tests)
    • test/unit/agent/swarm/swarm-graph.test.ts (12 tests)
    • test/unit/agent/swarm/swarm-merger.test.ts (13 tests)
    • test/unit/agent/swarm/swarm-router.test.ts (8 tests)
    • test/unit/agent/swarm/swarm-write-router.test.ts (10 tests)
    • test/unit/agent/swarm/config/swarm-config-schema.test.ts (16 tests)
    • test/unit/agent/swarm/validation/config-validator.test.ts (15 tests)
    • test/unit/agent/swarm/wizard/swarm-wizard.test.ts (13 tests)
    • test/unit/agent/swarm/cli/query-renderer.test.ts (6 tests)
    • test/unit/agent/swarm/tools/swarm-query-tool.test.ts (8 tests)
    • test/unit/agent/swarm/tools/swarm-store-tool.test.ts (7 tests)
    • test/unit/agent/swarm/system-prompt/swarm-state-contributor.test.ts (7 tests)
    • test/unit/agent/sandbox/tools-sdk-swarm.test.ts (8 tests)
      • 9 more test files (wizard, provider factory, detector, scaffolder, commands)
  • Key scenario(s) covered:
    • Multi-provider federated search with RRF fusion
    • DAG execution with enrichment chains (fan-in, cycle detection)
    • Query classification (factual/temporal/personal/relational)
    • Write routing (entity→GBrain, note→local-markdown, explicit override)
    • Search precision: stop words, AND-first, score floor, gap ratio at adapter + merger level
    • Sandbox tools.swarmQuery() / tools.swarmStore() with read-only mode enforcement
    • 35-scenario manual UAT across all providers (see docs/release-notes/swarm-memory-uat.md)

User-visible changes

  • New CLI commands:
    • brv swarm onboard — Interactive setup wizard with auto-detection
    • brv swarm status — Provider health, enrichment topology, write targets
    • brv swarm query "<text>" — Federated search (supports --format json, -n)
    • brv swarm curate "<content>" — Write routing (supports --provider, --format json)
  • New agent tools: swarm_query, swarm_store (auto-available when 2+ providers configured)
  • New sandbox methods: tools.swarmQuery(), tools.swarmStore() in code_exec
  • System prompt: Agent sees available swarm providers and write targets when configured
  • New config file: .brv/swarm/config.yaml (generated by wizard or manual)
  • New config fields: routing.min_rrf_score, routing.rrf_gap_ratio for precision tuning

Evidence

  • 288 unit tests passing: npx mocha --forbid-only "test/unit/agent/swarm/**/*.test.ts"
  • 35-scenario manual UAT documented: docs/release-notes/swarm-memory-uat.md
  • Typecheck clean: npm run typecheck
  • Lint clean on all swarm files

Checklist

  • Tests added or updated and passing (npm test)
  • Lint passes (npm run lint)
  • Type check passes (npm run typecheck)
  • Build succeeds (npm run build)
  • Commits follow Conventional Commits format
  • Documentation updated (if applicable)
  • No breaking changes (or clearly documented above)
  • Branch is up to date with main

Risks and mitigations

  • Risk: GBrain write operations take ~60s via Node execFile (PGLite WASM cold-start)
    • Mitigation: Timeout increased to 120s. Long-term: persistent gbrain server mode or native Postgres connection.
  • Risk: Search precision may filter out legitimate results in small corpora
    • Mitigation: Score floor (0.3) is intentionally lower than ByteRover's (0.45). All thresholds are tested. If too aggressive, min_rrf_score and rrf_gap_ratio can be tuned via config.
  • Risk: Honcho/Hindsight providers are temporarily disabled (stubs only)
    • Mitigation: Disabled at config level with clear "coming later" documentation. No dead code — stubs are minimal.

ngduyanhece and others added 8 commits April 11, 2026 16:04
…tus`

Implements the onboarding and health-check surface for the memory swarm system,
which will unify multiple memory providers (ByteRover, Obsidian, Honcho,
Hindsight, Local .md, GBrain) into a single intelligent memory layer.

Phase 1 delivers:
- IMemoryProvider interface and shared swarm domain types
- Zod config schema with snake_case YAML → camelCase TS mapping
- Config loader with env var resolution and tilde expansion
- Error accumulation pattern (MemorySwarmValidationError)
- Runtime provider validation (paths, credentials, connection strings)
- Provider auto-discovery (Obsidian vaults, .md folders, env vars)
- Interactive onboarding wizard with ESC back-navigation
- Config scaffolder with multi-folder merging and duplicate warnings
- `brv swarm onboard` — interactive setup wizard (oclif command)
- `brv swarm status` — provider health display with proactive suggestions

94 tests covering all modules.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ent tool

Implement the query pipeline for the memory swarm: query classification,
provider selection, parallel DAG execution with enrichment chains, and
Reciprocal Rank Fusion result merging.

Core engine:
- SwarmRouter: regex-based query classification (factual/temporal/personal/relational)
- SwarmGraph: topological DAG executor with multi-level enrichment chains and fan-in
- SwarmMerger: weighted RRF fusion with content deduplication
- SwarmCoordinator: orchestrates route → execute → merge, filters unhealthy providers
- ISwarmCoordinator interface with execute(), getActiveProviders(), getSummary()

Provider adapters:
- ByteRoverAdapter: wraps SearchKnowledgeService (writeSupported: false)
- ObsidianAdapter: MiniSearch index + wikilink graph expansion, honors ignorePatterns
- LocalMarkdownAdapter: per-folder MiniSearch, honors readOnly/followWikilinks/watchForChanges
- Provider factory builds adapters from config with name deduplication

CLI + agent integration:
- `brv swarm query` command with real SearchKnowledgeService in CLI mode
- swarm_query agent tool registered in tool registry
- SwarmStateContributor injects provider info into system prompt when >1 provider
- service-initializer wires coordinator into ToolProvider (fail-open on missing config,
  warns on invalid config)

191 unit tests covering all new code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allow users to control how providers feed context into each other
via enrichment edges in config.yaml. This wires the existing DAG
execution engine into user-facing config.

Config schema:
- New `enrichment.edges` section with `{from, to}` edge pairs
- Generic endpoints (e.g. `local-markdown`) expand to concrete
  provider IDs (`local-markdown:notes`) at both validation and
  execution time

Validation (expanded-graph aware):
- Cycle detection via DFS on the expanded edge set
- Self-edge rejection after generic expansion
- Provider existence check with prefix matching
- Folder-scoped typos (`local-markdown:missing`) rejected as errors
- Disabled-provider edges produce warnings only, not topology errors
- Enrichment errors excluded from cloud cascade note

Coordinator wiring:
- Reads config edges, expands generic endpoints, deduplicates,
  drops expansion-induced cycles, then passes to SwarmGraph
- Only enrichment topology errors block execution; provider-specific
  errors (bad paths, missing API keys) are logged as warnings and
  handled by health checks at runtime (degraded-mode preserved)

CLI + onboarding:
- `brv swarm status` displays enrichment topology in text and JSON
- `brv swarm onboard` scaffolder suggests default edges when 2+
  providers are enabled (byterover → obsidian, byterover → local-markdown)
- `brv swarm query` validates topology before executing

26 new tests covering schema parsing, edge validation, expansion,
dedup, cycle detection, coordinator wiring, status display, and
scaffolder defaults.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GBrain adapter (Phase 3a):
- New GBrainAdapter invokes gbrain CLI via subprocess with JSON output
- Binary resolution: explicit path → PATH → repoPath/src/cli.ts → workspace
  sibling (Bun fallback), with bun availability check in validation
- Three search modes: hybrid (vector+keyword+RRF), keyword-only, vector-only
- Auto-detection in provider-detector scans common locations for src/cli.ts
- Factory wiring creates adapter from config when gbrain.enabled
- Onboarding uses constrained select for search_mode (no typo risk)
- 15 new tests (adapter, factory, detector, scaffolder)

Temporarily disabled:
- Honcho + Hindsight removed from provider-detector, router matrix, and
  status display — adapters not yet implemented, re-enable in Phase 3b
- Budget step removed from onboarding wizard — no cloud cost tracking yet
- All disabled code marked with "re-enable in Phase 3" comments

Router matrix updated:
- All query types now route to available providers (byterover, obsidian,
  local-markdown, gbrain) since honcho/hindsight are disabled
- Personal queries no longer restricted to byterover-only

Enrichment defaults updated:
- Scaffolder generates byterover → gbrain edge when both enabled
- Onboarding enrichment explanation lists all downstream providers

237 unit tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use mkdtempSync for healthCheck tests so repoPath existence check
passes before the executor is invoked. Add test for nonexistent
repo path returning unavailable without calling the executor.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Write routing (Phase 3b):
- New swarm-write-router with classifyWrite() (entity/note/general) and
  selectWriteTarget() — regex-based, same pattern as query classification
- SwarmCoordinator.store() routes by explicit --provider, contentType hint,
  or auto-classification. Reuses healthCache from execute().
- `brv swarm curate` CLI command with --provider and --format flags
- swarm_store agent tool registered in tool registry
- System prompt shows write guidance when writable providers exist
- `brv swarm status` shows Write Targets section

Sandbox integration (Phase 3c):
- tools.swarmQuery(query, {limit?, scope?}) added to ToolsSDK
- tools.swarmStore({content, contentType?, provider?}) added to ToolsSDK
- swarmStore disabled in query (read-only) mode, swarmQuery allowed
- SandboxService.setSwarmCoordinator() setter with invalidateSandboxes()
- Conditional injection in CODE_EXEC factory (swarmCoordinator stays optional)
- ISandboxService interface extended with optional setSwarmCoordinator
- code_exec.txt tool description updated

Write target routing:
- entity content → GBrain (typed pages)
- note content → local-markdown (writable folder)
- general content → first healthy writable provider (config order)
- ByteRover and Obsidian are never write targets
- Explicit --provider overrides all classification

280 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Low precision in swarm queries (e.g. "project management" matching
"session_management") was caused by raw MiniSearch with no filtering in
the Local Markdown and Obsidian adapters, and no quality gate in the RRF
merger. This ports ByteRover's proven precision patterns to the swarm.

Adapter-level (T1/T2/T3):
- Stop-word filtering + AND-first search strategy with OR fallback
- Absolute score floor (0.3) rejects out-of-domain queries
- Gap ratio filter (0.75) trims low-scoring tail results
- Second gap-ratio pass (0.6) after wikilink expansion

Merger-level (T4/T5):
- minRRFScore: absolute RRF floor (configurable via routing config)
- rrfGapRatio: relative RRF gap filter (configurable, must be > 0)

Also increases GBrain subprocess timeout from 10s to 120s (PGLite WASM
cold-start via Node execFile takes ~60s for write operations).

288 tests passing (35 new).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@ngduyanhece ngduyanhece requested a review from danhdoan April 13, 2026 01:30
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2026

Claude finished @ngduyanhece's task in 4m 2s —— View job


PR Review: Federated Memory Swarm System

Tasks

  • Gather context and understand the PR
  • Review core domain types and interfaces
  • Review swarm infrastructure (coordinator, graph, router, merger)
  • Review adapters (ByteRover, Obsidian, GBrain, LocalMarkdown)
  • Review config system (schema, loader, validator)
  • Review CLI commands
  • Review agent tools and sandbox integration
  • Review test coverage
  • Post comprehensive review with inline comments

Overall Assessment: 4/5

This is a well-scoped, architecturally sound feature. The outside-in design is correct — IMemoryProvider is the right abstraction, the Zod schema-validated config with snake_case→camelCase normalization is robust, and the Kahn's algorithm DAG execution in SwarmGraph is implemented correctly. 288 unit tests, clean typecheck, clean lint. The PR does what it says it does and stays within its stated scope boundary.

Issues fall into three severity buckets:


Blocking / High

1. Unbounded loop in resolveUniqueFilenamelocal-markdown-adapter.ts:97 (inline)
A folder with many same-name files can spin the event loop indefinitely on any write. Add a cap and fall back to a timestamp suffix.

2. Synchronous execFileSync during GBrain adapter construction — gbrain-adapter.ts:62 (inline)
resolveGBrainBin calls execFileSync (5 s timeout) inside the constructor, blocking the event loop at startup. Resolution should be lazy or async.

3. Synchronous execFileSync inside async validateSwarmProvidersconfig-validator.ts:171 (inline)
Two sequential synchronous shell probes on the hot config-validation path. Switch to promisify(execFile).


Medium / Should Fix

4. Stale health cache — swarm-coordinator.ts:177 (inline)
healthCache starts as all-healthy and is never refreshed automatically. A provider that goes down mid-session will silently receive queries indefinitely. Needs TTL or failure-feedback from the graph.

5. 'creative' query type is unreachable — swarm-router.ts:22 (inline)
classifyQuery() never returns 'creative', yet the type and selection matrix entry exist. Either add a classifier or remove the dead type.

6. readOnly defaults to true for local-markdown folders — swarm-config-schema.ts:83 (inline)
Users who add a local-markdown folder without read_only: false will silently get a folder that rejects writes — including from the write router that prefers local-markdown for note type content. Confusing default.

7. Non-null assertion this.index! after ensureIndex()local-markdown-adapter.ts:190, obsidian-adapter.ts:153 (inline 1, inline 2)
Violates the project's "avoid ! non-null assertions" coding standard. Replace with an explicit null check returning [].


Low / Nit

8. Duplicated cycle-detection and resolveEndpointswarm-coordinator.ts:97 (inline)
hasCycleInEdges and resolveEndpoint are copy-pasted verbatim between the coordinator and the validator. Extract to a shared graph-utils.ts.

9. Duplicated ProviderInfo[] construction — swarm-coordinator.ts:258 (inline)
getActiveProviders() and getSummary() both build the same array. Extract a private helper.

10. Duplicated markdown scan/index/wikilink utilities — obsidian-adapter.ts:36 (inline)
extractWikilinks, findMarkdownFiles, buildIndexSignature, ScannedMarkdownFile, IndexedDoc, and the wikilink expansion block are copy-pasted verbatim. Follow-up PR candidate.

11. Enrichment passes raw content as "keywords" — swarm-graph.ts:57 (inline)
Semantically wrong label; no adapter currently consumes enrichment so the practical impact is zero, but this will matter in Phase 3. Document the intent.

12. Content-string deduplication in merger — swarm-merger.ts:44 (inline)
Exact-string dedup will miss near-duplicate excerpts from different adapters. Document the limitation.

13. Unimplemented schema sections (budget enforcement, optimization, provenance) — swarm-config-schema.ts:342 (inline)
monthlySpendCents is hardcoded to 0; edge learning and provenance are never triggered. Silent no-ops for configured settings. Add // Not yet implemented comments or emit warnings.

14. Broad env-var substitution in config loader — swarm-config-loader.ts:88 (inline)
resolveStringsDeep substitutes ${VAR} in every config string, including non-path fields. Low risk but worth narrowing to path/credential fields.

15. byterover-adapter.ts:41 indentation (inline)
Missing leading indent on public readonly id.


Scope Observations (not blocking)

  • SwarmCoordinator.writeSupported is hardcoded to false for ByteRoverAdapter — the write router correctly skips it, but the capabilities declaration says writeSupported: true in createDefaultCapabilities('byterover') in types.ts:174. These two sources disagree. Fix this →
  • GBrainAdapter.capabilities.localOnly is false even in keyword-only mode where no network calls are made. This causes the router to potentially exclude it from personal queries unnecessarily.

Comment thread src/agent/infra/swarm/adapters/byterover-adapter.ts
Comment thread src/agent/infra/swarm/adapters/local-markdown-adapter.ts
Comment thread src/agent/infra/swarm/adapters/local-markdown-adapter.ts Outdated
Comment thread src/agent/infra/swarm/adapters/obsidian-adapter.ts Outdated
Comment thread src/agent/infra/swarm/adapters/gbrain-adapter.ts
Comment thread src/agent/infra/swarm/swarm-coordinator.ts
Comment thread src/agent/infra/swarm/swarm-coordinator.ts
Comment thread src/agent/infra/swarm/swarm-coordinator.ts
Comment thread src/agent/infra/swarm/swarm-graph.ts
Comment thread src/agent/infra/swarm/swarm-router.ts
Comment thread src/agent/infra/swarm/config/swarm-config-schema.ts
Comment thread src/agent/infra/swarm/config/swarm-config-schema.ts
Comment thread src/agent/infra/swarm/swarm-merger.ts
Comment thread src/agent/infra/swarm/adapters/obsidian-adapter.ts
Comment thread src/agent/infra/swarm/validation/config-validator.ts
Comment thread src/agent/infra/swarm/config/swarm-config-loader.ts
cuongdo-byterover and others added 3 commits April 13, 2026 14:44
- Add default values for minRrfScore (0.005) and rrfGapRatio (0.5) in
  routing schema so RRF filters are active by default instead of being
  silently skipped when config omits them
- Add term-coverage validation on OR fallback in search-precision: when
  AND search returns no results, OR fallback now requires matched terms
  to exceed 50% of query terms, preventing partial-match noise
- Replace this.error() with logToStderr + exit(2) in swarm query/curate
  commands to suppress stack traces in dev mode
- Make curate.ts error handling consistent: replace remaining
  this.error() at result.success===false path with logToStderr+exit(2)
- Extract SearchResultWithTerms type alias to eliminate duplicate
  `as unknown as {queryTerms}` casts in search-precision.ts
- Add comment explaining why 2-word OR fallback is intentionally
  a near-no-op (AND failed exact match, OR may match via prefix/fuzzy)
- Rename misleading merger test and add 200-result scenario that
  actually exercises minRRFScore filtering at high ranks
cuongdo-byterover and others added 3 commits April 13, 2026 20:26
- Add resultCacheTtlMs (default 10s) to performance config schema
- Cache check at start of execute() returns instantly on hit
- LRU semantics via delete+re-set on cache hit (Map insertion order)
- Cache store after successful execution with max 20 entries
- Invalidation on store() (content changed) and refreshHealth()
  (provider availability changed)
- TTL 0 disables caching entirely
- 11 test cases covering hits, misses, TTL, invalidation, LRU eviction
- Add request.type and request.timeRange to cache key to prevent
  cross-type and cross-timerange cache collisions
- Deep-clone QueryResult items (including metadata) on both cache
  store and cache hit to prevent nested object mutation
- Skip cache store when resultCacheTtlMs=0 to avoid dead writes
  and unnecessary LRU eviction runs
- Mark healthCache and resultCache as readonly (mutated in-place
  via .set/.delete/.clear, never reassigned)
feat:[ENG-2073] add short-lived result cache in SwarmCoordinator
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2026

Claude finished @danhdoan's task in 2m 25s —— View job


PR Review: Federated Memory Swarm System (4th Review) — 4/5

Tasks

  • Gather context and understand the PR
  • Check blocking issues (local-markdown-adapter, gbrain-adapter, config-validator)
  • Check medium issues (swarm-coordinator, swarm-router, swarm-config-schema, adapters, types)
  • Check for new issues in new commits
  • Post comprehensive updated review

What's New Since Last Review

Two feature branches merged in (ENG-2073, ENG-2072 finalisation):

  • Short-lived result cache added to SwarmCoordinator (LRU with TTL) ✅
  • enrichment.keywordsenrichment.excerpts rename fully propagated ✅

No changes to any of the files flagged in the previous three reviews.


Blocking / High — All 3 Still Open

1. Unbounded while (true) loop — local-markdown-adapter.ts:97

while (true) {   // ← no upper bound
  const candidate = suffix === 0 ? `${baseName}.md` : `${baseName}-${suffix}.md`
  if (!existsSync(join(folderPath, candidate))) return candidate
  suffix++
}

Unchanged across all four reviews. A folder pre-loaded with slug.md, slug-1.md, … stalls the event loop indefinitely. Fix this →

2. Blocking execFileSync in GBrain constructor — gbrain-adapter.ts:62, 158

// resolveGBrainBin(), called from constructor:
execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})
// …
this.executor = executor ?? createDefaultExecutor(resolveGBrainBin(options))  // constructor

Still blocks the event loop up to 5 s on every adapter construction. Fix this →

3. Blocking execFileSync inside async validateSwarmProvidersconfig-validator.ts:175, 192

execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})  // line 175
execFileSync('bun', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})     // line 192

Two sequential synchronous probes — up to 10 s total — inside an already-async function. Fix this →


Medium / Should Fix — Status Update

4. Health cache — swarm-coordinator.ts:214–217 — Partially addressed ✅

execute() now filters with healthCache.get(p.id) !== false (undefined-as-healthy) and the invariant is documented in a comment. Meaningful improvement.

Remaining gap: cache starts all-true, is never automatically refreshed, and providers that fail mid-session stay in rotation forever. The comment documents the invariant but not the manual-refresh contract for callers. A JSDoc on refreshHealth() saying "must be called periodically for sessions longer than X" would complete this.

5. Unreachable 'creative' query type — swarm-router.ts:32 — Still open 🟡

classifyQuery() returns only 'temporal' | 'personal' | 'relational' | 'factual'. SELECTION_MATRIX still has a creative entry. Dead code that misleads future contributors. Fix this →

6. readOnly defaults to true in schema — swarm-config-schema.ts:83 — Still open 🟡

readOnly: z.boolean().optional().default(true),  // ← config-parsed folders are read-only by default

Users who add a local-markdown folder via config without read_only: false will get silent write failures from brv swarm curate. The write router routes note writes to local-markdown first — those fail out of the box. Fix this →

7. Non-null assertions this.index!local-markdown-adapter.ts:190, obsidian-adapter.ts:153 — Still open 🟡

Violates the project's "avoid ! non-null assertions" coding standard. Replace with explicit guard: Fix this →

8. writeSupported mismatch in createDefaultCapabilitiestypes.ts:174 — Still open 🟡

createDefaultCapabilities('byterover') returns writeSupported: true but ByteRoverAdapter.capabilities.writeSupported is false (line 39 of byterover-adapter.ts). Any caller using the factory instead of reading from the adapter instance gets the wrong value. Fix this →


Low / Nit — Carry-over (unchanged)

  • byterover-adapter.ts:41public readonly id = 'byterover' is missing its leading two-space indent. Flagged in all four reviews.
  • Duplicated cycle-detectionhasCycleInEdges in swarm-coordinator.ts:98 and hasCycle in config-validator.ts:278 are identical DFS implementations. Extract to graph-utils.ts.
  • Duplicated ProviderInfo[] constructiongetActiveProviders() (line 294) and getSummary() (line 306) in coordinator both build the same array. Extract a private helper.
  • Duplicated markdown utilitiesextractWikilinks, findMarkdownFiles, etc. are copy-pasted between obsidian-adapter.ts and local-markdown-adapter.ts. Follow-up PR candidate.
  • Content-string deduplicationswarm-merger.ts:44 uses exact result.content match. Near-duplicates from different adapters won't merge. Add a JSDoc note.
  • Unimplemented schema sectionsbudget.perProvider, optimization.edgeLearning, provenance are configured but never executed. Add // Not yet implemented comments.
  • Broad env-var substitutionresolveStringsDeep in loader substitutes ${VAR} in all config strings, not just paths/credentials.

Summary

# Issue File Status
1 Unbounded loop in resolveUniqueFilename local-markdown-adapter.ts:97 🔴 Open (4 reviews)
2 Blocking execFileSync in constructor gbrain-adapter.ts:62,158 🔴 Open (4 reviews)
3 Blocking execFileSync in async validator config-validator.ts:175,192 🔴 Open (4 reviews)
4 Stale health cache — no TTL/auto-refresh swarm-coordinator.ts:214 🟡 Partially addressed
5 Unreachable 'creative' type swarm-router.ts:32 🟡 Open
6 readOnly default mismatch schema→adapter swarm-config-schema.ts:83 🟡 Open
7 Non-null this.index! assertions *-adapter.ts:190, 153 🟡 Open
8 writeSupported mismatch in createDefaultCapabilities types.ts:174 🟡 Open

The three blocking issues have been flagged in every review cycle and are real bugs that must be fixed before merge. The --explain flag and result cache are welcome additions but they don't change the risk profile of the unaddressed issues.

cuongdo-byterover and others added 7 commits April 14, 2026 10:00
- Change selectWriteTarget() to return null instead of throwing when
  no writable providers are available
- Add optional ICurateService to SwarmCoordinator constructor
- Add fallbackToByterover() that routes content to the curation
  pipeline (snippets) when no external provider can accept the write
- Add fallback?: boolean to SwarmStoreResult type
- Update CLI curate output to show fallback message
- 5 new tests: fallback with/without curateService, fallback on
  curate failure, no fallback for explicit --provider, normal write
  without fallback flag
Resolved conflicts in swarm-coordinator.ts:
- Constructor: keep both curateService (ENG-2075) and resultCacheTtlMs (ENG-2073)
- Private methods: keep fallbackToByterover (ENG-2075) alongside
  buildCacheKey and evictIfOverSize (ENG-2073)
…teService

The fallback now shells out to `brv curate --detach` — same path as the
CLI command. This goes through the daemon + LLM agent for proper domain
detection, topic routing, and content structuring, instead of manually
constructing CurateOperation objects with hardcoded paths.

Also addresses PR review comments:
- Fix stale @throws JSDoc in selectWriteTarget (now returns null)
- Remove fallback: false from no-curateService path (use undefined)
- Remove CurateService dependency from coordinator and CLI
- Use --format json for brv curate subprocess, parse logId from JSON
  instead of capturing human-readable stdout
- Extract execBrvCurate as a standalone function, inject via constructor
  (CurateFallbackFn) so tests can mock without ES module stubbing issues
- Add proper unit tests with sinon stubs for both success and failure
  fallback paths
- Fix stale @throws JSDoc in selectWriteTarget (now @returns null)
- Move imports to top of file, place execBrvCurate after (fix lint)
- Add error field to BrvCurateResult type, propagate parsed.error
  into SwarmStoreResult when brv curate returns success: false
- Remove export from execBrvCurate (only used internally, tests
  inject via CurateFallbackFn constructor param)
- Handle empty id in CLI output (no trailing "as ")

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use util.promisify(execFile) instead of manual Promise wrapper
- Export BrvCurateResult type so CurateFallbackFn is complete for
  external consumers
- Fix stale selectWriteTarget docstring (no longer says callers
  must pre-filter providers)
feat:[ENG-2075] add ByteRover context tree as write fallback
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Claude finished @danhdoan's task in 3m 9s —— View job


PR Review: Federated Memory Swarm System (5th Review) — 4/5

Tasks

  • Gather context and understand the PR
  • Check blocking issues status (local-markdown-adapter, gbrain-adapter, config-validator)
  • Check medium issues status (swarm-coordinator, swarm-router, swarm-config-schema, adapters, types)
  • Check new commits for new issues
  • Post comprehensive updated review

What's New Since Last Review

The ENG-2075 feature merged since the 4th review:

  • SwarmCoordinator.store() now calls fallbackToByterover() when no external writable provider is available
  • fallbackToByterover() shells out to brv curate --detach --format json via execFileAsync (correctly async — good)
  • SwarmCoordinator now depends on an injectable curateFallback function (testable — good)
  • brv swarm curate CLI now shows a distinct "fallback" message when writing via ByteRover
  • SwarmWriteRouter.selectWriteTarget() now returns null instead of throwing (correct JSDoc fix)

Blocking / High — All 3 Still Open (5 consecutive reviews)

1. Unbounded while (true) loop — local-markdown-adapter.ts:97

while (true) {   // ← no upper bound; unchanged since review 1
  const candidate = suffix === 0 ? `${baseName}.md` : `${baseName}-${suffix}.md`
  if (!existsSync(join(folderPath, candidate))) return candidate
  suffix++
}

Fix this →

2. Blocking execFileSync in GBrain constructor — gbrain-adapter.ts:62, 158

// Called from the constructor at line 158:
this.executor = executor ?? createDefaultExecutor(resolveGBrainBin(options))
// Inside resolveGBrainBin:
execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})

Blocks event loop up to 5 s on every adapter construction. Fix this →

3. Blocking execFileSync inside async validateSwarmProvidersconfig-validator.ts:175, 192

execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})  // line 175
execFileSync('bun', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})     // line 192

Fix this →


New Issues from ENG-2075

9. Large content passed as CLI argument — swarm-coordinator.ts:29

await execFileAsync('brv', ['curate', '--detach', '--format', 'json', content], ...)

content is the raw store payload with no size limit. At scale (large notes, documents) this can exceed ARG_MAX (~256 KB on Linux, ~2 MB on macOS), causing E2BIG spawn failures. Content should be passed via stdin instead:

const {stdout} = await execFileAsync('brv', ['curate', '--detach', '--format', 'json', '--stdin'], {
  encoding: 'utf8',
  input: content,
  timeout: 30_000,
})

Fix this →

10. as type assertion in error handler — swarm-coordinator.ts:34

const err = error as {message: string; stderr?: string}   // ← violates "avoid as Type assertions"
throw new Error(err.stderr?.trim() || err.message)        // ← || triggers on empty string

Use instanceof Error narrowing and ?? instead:

if (error instanceof Error) throw new Error((error as NodeJS.ErrnoException).stderr?.trim() ?? error.message)
throw new Error(String(error))

Fix this →

11. as type assertion in SwarmCurate error handler — curate.ts:77, 79

this.log(JSON.stringify({error: (error as Error).message, success: false}))
this.logToStderr(`Error: ${(error as Error).message}`)

Both violate "avoid as Type assertions". Replace with error instanceof Error ? error.message : String(error). Fix this →


Medium / Should Fix — Status Update

4. Stale health cache — Partially addressed ✅

execute() filters providers with healthCache.get(p.id) !== false. Good. Still starts all-true, no auto-refresh. The manual refreshHealth() contract is not documented in the class JSDoc. Also: SwarmCurate CLI now calls coordinator.refreshHealth() before store() — this is the right pattern, but the same pattern is absent from SwarmQuery CLI.

5. Unreachable 'creative' query type — swarm-router.ts:32 — Still open 🟡

classifyQuery() returns only 'temporal' | 'personal' | 'relational' | 'factual'. SELECTION_MATRIX.creative is dead code. Fix this →

6. readOnly defaults to true in schema — swarm-config-schema.ts:83 — Still open 🟡

Users who add a local-markdown folder via config without read_only: false silently get a read-only folder. brv swarm curate will fail at the write-routing step. Fix this →

7. Non-null assertions this.index!local-markdown-adapter.ts:190, obsidian-adapter.ts:153 — Still open 🟡

Violates the project's "avoid ! non-null assertions" coding standard. Fix this →

8. writeSupported mismatch in createDefaultCapabilitiestypes.ts:174 — Still open 🟡

createDefaultCapabilities('byterover') returns writeSupported: true but ByteRoverAdapter.capabilities.writeSupported is false. Fix this →


Low / Nit — Carry-over

  • Indentationbyterover-adapter.ts:41: public readonly id = 'byterover' is missing its leading two-space indent. Flagged in all five reviews.
  • Indentationcurate.ts:14: public static description is also missing its leading indent (new nit from ENG-2075).
  • Duplicated cycle-detectionhasCycleInEdges in swarm-coordinator.ts:124 and hasCycle in config-validator.ts:278 are identical DFS implementations. Extract to graph-utils.ts.
  • Duplicated ProviderInfo[] constructiongetActiveProviders() (line 323) and getSummary() (line 336) in coordinator both build the same array. Extract a private helper.
  • Duplicated markdown utilitiesextractWikilinks, findMarkdownFiles, etc. copy-pasted between obsidian and local-markdown adapters. Follow-up PR candidate.
  • Content-string deduplicationswarm-merger.ts:44 uses exact result.content match. Add a JSDoc note.
  • Unimplemented schema sectionsbudget.perProvider, optimization.edgeLearning, provenance are configured but never executed. Add // Not yet implemented comments.
  • Broad env-var substitutionresolveStringsDeep in loader substitutes ${VAR} in all config strings, not just paths/credentials.

Summary

# Issue File Status
1 Unbounded loop in resolveUniqueFilename local-markdown-adapter.ts:97 🔴 Open (5 reviews)
2 Blocking execFileSync in constructor gbrain-adapter.ts:62,158 🔴 Open (5 reviews)
3 Blocking execFileSync in async validator config-validator.ts:175,192 🔴 Open (5 reviews)
4 Stale health cache — no TTL/auto-refresh swarm-coordinator.ts 🟡 Partially addressed
5 Unreachable 'creative' type swarm-router.ts:32 🟡 Open
6 readOnly default mismatch schema→adapter swarm-config-schema.ts:83 🟡 Open
7 Non-null this.index! assertions *-adapter.ts:190, 153 🟡 Open
8 writeSupported mismatch in createDefaultCapabilities types.ts:174 🟡 Open
9 Large content as CLI arg — E2BIG risk swarm-coordinator.ts:29 🔴 New (ENG-2075)
10 as assertion + || in error handler swarm-coordinator.ts:34 🟡 New (ENG-2075)
11 as assertion in SwarmCurate error handler curate.ts:77,79 🟡 New (ENG-2075)

The three original blocking issues have been flagged in every review since the PR opened. Issues #9#11 are new from the ENG-2075 merge, with #9 being a new blocking issue: E2BIG errors are silent in CI but will surface on real machines storing large documents.

cuongdo-byterover and others added 6 commits April 14, 2026 15:30
Adds memory-wiki as the 7th swarm provider, connecting to OpenClaw's
Memory Wiki plugin via the `openclaw wiki search/get` CLI subprocess.

- Add 'memory-wiki' to PROVIDER_TYPES and createDefaultCapabilities
- Add MemoryWikiProviderSchema to config schema (openclaw_path,
  vault_path, search_corpus)
- Create MemoryWikiAdapter implementing IMemoryProvider with injectable
  executor for testability (same pattern as GBrain)
- Wire in provider factory, DEFAULT_WEIGHTS (0.8), selection matrix
  (all types except personal)
- Add [wiki] source label in whiteBright for CLI output
- Add memory-wiki to config validator (getEnabledProviderIds,
  isConfiguredProvider)
- Add memory-wiki to swarm status display
- Score normalization: BM25 unbounded → [0,1) via score/(1+score)
- Read-only provider (wiki is compiled, not directly writable)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical bugs:
- Fix inverted capabilities in createDefaultCapabilities (semanticSearch
  false not true, writeSupported true not false, avgLatencyMs 60 not 150)
- Add memory-wiki to LOCAL_PROVIDERS set (was misclassified as cloud)
- Add mkdirSync in store() to handle fresh vaults without concepts/
  or entities/ directories

Correctness:
- Add validateMemoryWiki in config validator (checks vault_path exists)
- Fix status count: include memoryWiki in enabledCount, dynamic
  denominator instead of hardcoded /4
- Add memory-wiki to write targets display
- Show vault_path instead of hardcoded 'OpenClaw wiki' in status
- Fix RRF weight to 0.9 (was reverted to 0.8 by merge)

Content extraction:
- extractContentSection now checks openclaw:wiki:content markers first
  (recognizes pages written by store()), then code blocks, then
  frontmatter stripping

Tests:
- Add createDefaultCapabilities('memory-wiki') test
- Add isLocalProvider/isCloudProvider tests for memory-wiki
… adapter

- Fix indentation misalignment on private readonly boostFresh field
- Replace writeFileSync with async writeFile from fs/promises to avoid
  blocking the event loop in the async store() method
- Fix indentation: boostFresh field in adapter, errors.push in validator
- Remove redundant vaultPath truthy guard in factory (required field)
- Fold memory-wiki isLocalProvider/isCloudProvider assertions into
  existing describe blocks instead of standalone section
feat:[ENG-2080] Integrate Memory Wiki with Swarm memory
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Claude finished @danhdoan's task in 3m 44s —— View job


PR Review: Federated Memory Swarm System (6th Review) — 4/5

Tasks

  • Gather context and understand the PR
  • Check new commits since 5th review (ENG-2080: Memory Wiki adapter)
  • Verify blocking issues status
  • Verify medium issues status
  • Check for new issues in ENG-2080 changes
  • Post comprehensive updated review

What's New Since Last Review

ENG-2080 merged since the 5th review:

  • MemoryWikiAdapter added — reads from OpenClaw wiki vault (digest-based MiniSearch index with freshness/kind scoring)
  • memory-wiki added to provider factory, schema, SELECTION_MATRIX, and write-router fallback chain
  • Async writeFile used (good — no sync I/O in the new write path)
  • query() correctly uses a if (!this.index) return [] null guard (good — this is the pattern the other two adapters should follow)

Blocking / High — All 4 Still Open + 1 New

1. Unbounded while (true) loop — local-markdown-adapter.ts:97 — 🔴 Open (6 reviews)

while (true) {   // ← no cap after 6 reviews
  const candidate = suffix === 0 ? `${baseName}.md` : `${baseName}-${suffix}.md`
  if (!existsSync(join(folderPath, candidate))) return candidate
  suffix++
}

Fix this →

2. Blocking execFileSync in GBrain constructor — gbrain-adapter.ts:62, 158 — 🔴 Open (6 reviews)

// Inside resolveGBrainBin(), called from constructor at line 158:
execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})
this.executor = executor ?? createDefaultExecutor(resolveGBrainBin(options))

Fix this →

3. Blocking execFileSync inside async validateSwarmProvidersconfig-validator.ts:175, 192 — 🔴 Open (6 reviews)

execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})  // line 175
execFileSync('bun', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})     // line 192

Fix this →

4. Large content passed as CLI argument — swarm-coordinator.ts:29 — 🔴 Open (2 reviews)

;({stdout} = await execFileAsync('brv', ['curate', '--detach', '--format', 'json', content], {
  encoding: 'utf8',
  timeout: 30_000,
}))

content with no size limit will hit E2BIG on large documents (~256 KB on Linux). Pass via stdin instead.

Fix this →

5. New unbounded loop in memory-wiki-adapter.ts:210 — 🔴 New (ENG-2080)

ENG-2080 introduced the exact same unbounded-loop pattern that has been flagged in local-markdown-adapter.ts for six reviews:

// memory-wiki-adapter.ts:210
let suffix = 1
while (existsSync(filePath)) {   // ← no upper bound
  filename = `${slug}-${suffix}.md`
  filePath = join(dirPath, filename)
  suffix++
}

A wiki vault pre-loaded with slug.md, slug-1.md, slug-N.md will stall the event loop indefinitely on any store() call.

Fix this →


Medium / Should Fix — Status Update

6. Unreachable 'creative' query type — swarm-router.ts:15–38 — 🟡 Open (6 reviews)

ENG-2080 added memory-wiki to the creative entry in SELECTION_MATRIX, which means we now have even more dead code attached to an unreachable branch:

creative: ['byterover', 'obsidian', 'local-markdown', 'gbrain', 'memory-wiki'],  // ← still unreachable

classifyQuery() still only returns 'temporal' | 'personal' | 'relational' | 'factual'. Fix this →

7. readOnly defaults to true in schema — swarm-config-schema.ts:83 — 🟡 Open (6 reviews)

readOnly: z.boolean().optional().default(true),  // ← config-parsed folders are read-only by default

Users who add a local-markdown folder via config without read_only: false will not see it in the write-target candidates (the adapter sets writeSupported: !this.readOnly). Write routing falls through to memory-wiki or ByteRover fallback. Users don't know why their local-markdown folder isn't being written to. Fix this →

8. Non-null assertions this.index!local-markdown-adapter.ts:190, obsidian-adapter.ts:153 — 🟡 Open (6 reviews)

Violates the project's "avoid ! non-null assertions" standard. Notably, memory-wiki-adapter.ts (ENG-2080) gets this right:

// memory-wiki-adapter.ts:152 — correct pattern ✅
if (!this.index) {
  return []
}

// local-markdown-adapter.ts:190 — still wrong ❌
const precisionResults = searchWithPrecision(this.index!, request.query, {maxResults})

// obsidian-adapter.ts:153 — still wrong ❌
const precisionResults = searchWithPrecision(this.index!, request.query, {maxResults})

Fix this →

9. writeSupported mismatch in createDefaultCapabilities('byterover')types.ts:174 — 🟡 Open (5 reviews)

createDefaultCapabilities('byterover') returns writeSupported: true but ByteRoverAdapter.capabilities.writeSupported is false (byterover-adapter.ts:39). Fix this →

10. as assertion + || in error handler — swarm-coordinator.ts:33–35 — 🟡 Open (2 reviews)

const err = error as {message: string; stderr?: string}   // ← violates "avoid as Type assertions"
throw new Error(err.stderr?.trim() || err.message)        // ← || triggers on empty string ''

Fix this →

11. as assertion in SwarmCurate error handler — curate.ts:77, 79 — 🟡 Open (2 reviews)

this.log(JSON.stringify({error: (error as Error).message, success: false}))
this.logToStderr(`Error: ${(error as Error).message}`)

Fix this →


Low / Nit — Carry-over

  • byterover-adapter.ts:41public readonly id = 'byterover' is missing its leading two-space indent. Flagged in all six reviews.
  • curate.ts:14public static description is also missing leading indent (flagged since ENG-2075).
  • Duplicated cycle-detectionhasCycleInEdges in swarm-coordinator.ts:125 and hasCycle in config-validator.ts:278 are identical DFS implementations. Extract to graph-utils.ts.
  • Duplicated ProviderInfo[] constructiongetActiveProviders() and getSummary() both build the same array. Extract a private helper.
  • Duplicated markdown utilitiesextractWikilinks, findMarkdownFiles, etc. copy-pasted between obsidian and local-markdown adapters. Follow-up PR candidate.
  • Content-string deduplicationswarm-merger.ts:44 uses exact result.content match. Add a JSDoc note.
  • Unimplemented schema sectionsbudget.perProvider, optimization.edgeLearning, provenance are configured but never executed. Add // Not yet implemented comments.
  • Broad env-var substitutionresolveStringsDeep in loader substitutes ${VAR} in all config strings, not just paths/credentials.

Summary

# Issue File Status
1 Unbounded loop in resolveUniqueFilename local-markdown-adapter.ts:97 🔴 Open (6 reviews)
2 Blocking execFileSync in constructor gbrain-adapter.ts:62,158 🔴 Open (6 reviews)
3 Blocking execFileSync in async validator config-validator.ts:175,192 🔴 Open (6 reviews)
4 Large content as CLI arg — E2BIG risk swarm-coordinator.ts:29 🔴 Open (2 reviews)
5 Unbounded loop in memory-wiki store() memory-wiki-adapter.ts:210 🔴 New (ENG-2080)
6 Unreachable 'creative' type swarm-router.ts:15 🟡 Open (6 reviews)
7 readOnly default mismatch schema→adapter swarm-config-schema.ts:83 🟡 Open (6 reviews)
8 Non-null this.index! assertions *-adapter.ts:190, 153 🟡 Open (6 reviews)
9 writeSupported mismatch in createDefaultCapabilities types.ts:174 🟡 Open (5 reviews)
10 as assertion + || in error handler swarm-coordinator.ts:33 🟡 Open (2 reviews)
11 as assertion in SwarmCurate curate.ts:77,79 🟡 Open (2 reviews)

The memory-wiki-adapter itself is well-written — the freshness/kind scoring, the digest-based index signature, and the correct null guard on this.index are all good. Issues #1#3 have been open since the first review. At this point the three synchronous-I/O issues (#2, #3) and the unbounded loops (#1, #5) are the clearest technical debt preventing a safe merge.

cuongdo-byterover and others added 3 commits April 14, 2026 17:07
Blocking (5 fixes):
- #1: Bound resolveUniqueFilename loop to MAX_SUFFIX=10000 with
  timestamp fallback (local-markdown-adapter)
- #2: Make GBrain bin resolution lazy — execFileSync no longer runs
  in constructor, deferred to first executor use
- #3: Replace execFileSync with async execFileAsync in config-validator
  (validateGBrain is now async)
- #4: Add MAX_ARG_LENGTH guard (200KB) in execBrvCurate to prevent
  E2BIG on large content
- #5: Bound memory-wiki store() filename loop with same pattern

Medium (6 fixes):
- #6: Remove unreachable 'creative' from QueryType union and
  SELECTION_MATRIX
- #8: Replace this.index! non-null assertions with explicit null
  guards in obsidian and local-markdown adapters
- #9: Fix byterover writeSupported: true → false in
  createDefaultCapabilities (matches adapter)
- #10: Replace 'as' type assertion with instanceof narrowing in
  execBrvCurate error handler
- #11: Replace 'as Error' assertions with instanceof checks in
  swarm curate and query CLI commands
- Stale JSDoc: remove 'creative' from router comment

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ments

Prevent YAML title injection by using JSON.stringify instead of raw
interpolation. Add clarifying comments for null guard and sync probe.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Claude finished @danhdoan's task in 3m 28s —— View job


PR Review: Federated Memory Swarm System (7th Review) — 4.5/5

Tasks

  • Gather context and understand the PR
  • Check blocking issues status (local-markdown-adapter, gbrain-adapter, config-validator, swarm-coordinator, memory-wiki-adapter)
  • Check medium issues status (swarm-router, swarm-config-schema, adapters, types, error handlers)
  • Check for new issues in new commits
  • Post comprehensive updated review

What's New Since Last Review

Two commits landed since the 6th review:

  • fix(swarm): resolve all blocking and medium PR review issues (32c541a) — 11 targeted fixes across 6 files
  • fix: harden memory-wiki YAML injection and add reviewer-requested comments (ce4a23b) — title: ${JSON.stringify(title)} in memory-wiki YAML front-matter; clarifying comment on lazy executor in gbrain-adapter

Previously Blocking — All Resolved ✅

# Issue Status
1 Unbounded loop in resolveUniqueFilename ✅ Fixed — MAX_SUFFIX = 10_000 + timestamp fallback (local-markdown-adapter.ts:97)
2 Blocking execFileSync in GBrain constructor ✅ Fixed — lazy get executor() getter; resolveGBrainBin deferred to first use (gbrain-adapter.ts:164)
3 Blocking execFileSync in async validator ✅ Fixed — await execFileAsync(...) in validateGBrain (config-validator.ts:178,195)
4 Large content as CLI arg — E2BIG risk ✅ Mitigated — MAX_ARG_LENGTH = 200_000 guard added; throws a clear error if exceeded (swarm-coordinator.ts:26)
5 Unbounded loop in memory-wiki store() ✅ Fixed — while (existsSync(filePath) && suffix <= MAX_SUFFIX) + timestamp fallback (memory-wiki-adapter.ts:212)

Previously Medium — Nearly All Resolved

# Issue Status
6 Unreachable 'creative' query type ✅ Fixed — 'creative' removed from QueryType union and SELECTION_MATRIX entirely
7 readOnly defaults to true in LocalMarkdownFolderSchema 🟡 Still open — see below
8 Non-null this.index! assertions ✅ Fixed — both local-markdown-adapter.ts and obsidian-adapter.ts now use explicit if (!this.index) return []
9 writeSupported mismatch in createDefaultCapabilities ✅ Fixed — createDefaultCapabilities('byterover') now returns writeSupported: false
10 as assertion + || in execBrvCurate ✅ Fixed — instanceof Error narrowing + ?? operator
11 as Error in SwarmCurate error handler ✅ Fixed — error instanceof Error ? error.message : String(error)

Remaining Open Issue

readOnly defaults to trueswarm-config-schema.ts:83 — 🟡 Open (7 reviews)

// LocalMarkdownFolderSchema line 83:
readOnly: z.boolean().optional().default(true),  // ← still defaults to read-only

This has been flagged every review. The adapter derives writeSupported from !this.readOnly, so any folder added via config without read_only: false will silently have writeSupported: false. The write router routes note writes to local-markdown first — they will fail out of the box for most users.

The fix is one character: default(true)default(false). The ObsidianProviderSchema at line 64 can keep default(true) since Obsidian hardcodes writeSupported: false anyway. Fix this →


New Issues from Latest Commits

New: YAML injection hardened in memory-wiki-adapter.ts ✅ (ce4a23b)

title: ${JSON.stringify(title)} prevents YAML front-matter injection from note titles containing :, #, or ---. Well-handled.

Residual: execFileSync still present inside lazy resolveGBrainBingbrain-adapter.ts:62

// Comment says "runs lazily on first executor access" — correct.
// But the call is still synchronous when it does run:
execFileSync('gbrain', ['--version'], {encoding: 'utf8', stdio: 'pipe', timeout: 5000})

The lazy getter pattern solves the construction-time blocking issue (#2 above). However, execFileSync will still block the event loop for up to 5 s on the first query/store call. This is significantly less impactful than blocking at startup, and the comment correctly documents the behavior. Flagging as a low-priority follow-up rather than a blocker — acceptable for merge. Follow-up fix →


Low / Nit — Carry-over (7 reviews)

  • byterover-adapter.ts:41public readonly id = 'byterover' is still missing its leading two-space indent. Flagged in every review.
  • curate.ts:14public static description = ... is also missing leading indent (flagged since ENG-2075).
  • Duplicated cycle-detectionhasCycleInEdges in swarm-coordinator.ts:135 and hasCycle in config-validator.ts remain identical DFS implementations. Follow-up PR.
  • Duplicated ProviderInfo[] constructiongetActiveProviders() and getSummary() in coordinator still build the same array. Follow-up PR.
  • Duplicated markdown utilitiesextractWikilinks, findMarkdownFiles, etc. remain copy-pasted between obsidian and local-markdown adapters. Follow-up PR.
  • Content-string deduplicationswarm-merger.ts:44 uses exact result.content match. Still missing JSDoc note.
  • Unimplemented schema sectionsbudget.perProvider, optimization.edgeLearning, provenance still have no // Not yet implemented comments.
  • Broad env-var substitutionresolveStringsDeep in loader still substitutes ${VAR} in all config strings.

Summary

# Issue File Status
1 Unbounded loop in resolveUniqueFilename local-markdown-adapter.ts:97 ✅ Resolved
2 Blocking execFileSync in constructor gbrain-adapter.ts:62,158 ✅ Resolved (lazy)
3 Blocking execFileSync in async validator config-validator.ts:175,192 ✅ Resolved
4 Large content as CLI arg — E2BIG risk swarm-coordinator.ts:29 ✅ Mitigated (200KB guard)
5 Unbounded loop in memory-wiki store() memory-wiki-adapter.ts:210 ✅ Resolved
6 Unreachable 'creative' type swarm-router.ts ✅ Resolved
7 readOnly default true in LocalMarkdownFolderSchema swarm-config-schema.ts:83 🟡 Still open
8 Non-null this.index! assertions *-adapter.ts ✅ Resolved
9 writeSupported mismatch in createDefaultCapabilities types.ts:174 ✅ Resolved
10–11 as assertions in error handlers multiple ✅ Resolved
New YAML injection in memory-wiki memory-wiki-adapter.ts ✅ Fixed in ce4a23b

One line change separates this PR from merge-ready: default(true)default(false) in swarm-config-schema.ts:83. All 10 other previously-flagged issues have been resolved. The PR is well-tested (288 unit tests), typecheck clean, lint clean, and architecturally sound. Strong work addressing the backlog of review comments.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Claude finished @leehpham's task in 2m 10s —— View job


PR Review: Federated Memory Swarm System (8th Review) — 4.5/5

Tasks

  • Gather context and understand the PR
  • Check remaining open issue (readOnly default in swarm-config-schema.ts:83)
  • Check for new commits and new issues since 7th review
  • Post comprehensive updated review

What's New Since Last Review

One merge commit landed since the 7th review:

  • 217c92d — Merge main into proj/swarm-memory (brings in ENG-2050: auto-sync .gitignore on upgrade). No swarm code touched — the merge is clean and introduces no regressions.

Blocking / High — All Resolved ✅

All five previously blocking issues remain fixed:

# Issue Status
1 Unbounded loop in resolveUniqueFilename MAX_SUFFIX = 10_000 + timestamp fallback
2 Blocking execFileSync in GBrain constructor ✅ Lazy private get executor() getter (gbrain-adapter.ts:164)
3 Blocking execFileSync in async validator await execFileAsync(...) throughout
4 Large content as CLI arg — E2BIG risk MAX_ARG_LENGTH = 200_000 guard
5 Unbounded loop in memory-wiki store() while (... && suffix <= MAX_SUFFIX) + timestamp fallback

Still Open: One Medium Issue (8 consecutive reviews)

readOnly defaults to true in LocalMarkdownFolderSchemaswarm-config-schema.ts:83

// LocalMarkdownFolderSchema — swarm-config-schema.ts:83
readOnly: z.boolean().optional().default(true),   // ← still defaults to read-only

This is unchanged since the first review. The adapter derives writeSupported from !this.readOnly, so any local-markdown folder added via config without read_only: false will silently have writeSupported: false. The write router routes note writes to local-markdown first — they will fail out of the box for most users who don't know to set the flag.

The ObsidianProviderSchema at line 64 can keep default(true) since Obsidian hardcodes writeSupported: false regardless. Fix this →


Previously Medium — All Resolved ✅

# Issue Status
6 Unreachable 'creative' query type ✅ Removed from QueryType and SELECTION_MATRIX
8 Non-null this.index! assertions ✅ Both adapters use explicit if (!this.index) return []
9 writeSupported mismatch in createDefaultCapabilities byterover case now returns writeSupported: false
10–11 as assertions in error handlers instanceof Error narrowing + ?? operator

Low / Nit — Carry-over

  • byterover-adapter.ts:41public readonly id = 'byterover' is still missing its leading two-space indent. Flagged in all eight reviews.
  • curate.ts:14public static description = ... is also missing leading indent (flagged since review 5).
  • Duplicated cycle-detectionhasCycleInEdges in swarm-coordinator.ts and hasCycle in config-validator.ts remain identical DFS implementations. Follow-up PR.
  • Duplicated ProviderInfo[] constructiongetActiveProviders() and getSummary() in coordinator still build the same array. Follow-up PR.
  • Duplicated markdown utilitiesextractWikilinks, findMarkdownFiles, etc. copy-pasted between obsidian and local-markdown adapters. Follow-up PR.
  • Content-string deduplicationswarm-merger.ts:48 uses exact result.content match for dedup. Near-duplicates from different adapters won't merge. Worth a JSDoc note.
  • Unimplemented schema sectionsbudget, optimization.edgeLearning, provenance are configured but never executed. Still no // Not yet implemented comments.
  • Broad env-var substitutionresolveStringsDeep in loader still substitutes ${VAR} in all config strings, not just paths/credentials.

Summary

# Issue File Status
1–5 All blocking issues multiple ✅ Resolved
6, 8–11 All medium issues multiple ✅ Resolved
7 readOnly default true in LocalMarkdownFolderSchema swarm-config-schema.ts:83 🟡 Still open (8 reviews)

The merge from main is clean. The PR remains one character away from merge-ready: default(true)default(false) on swarm-config-schema.ts:83. Everything else has been addressed. The code is well-tested, typecheck-clean, lint-clean, and architecturally sound.

@leehpham leehpham merged commit 1c5bdd0 into main Apr 14, 2026
6 checks passed
@leehpham leehpham deleted the proj/swarm-memory branch April 14, 2026 11:01
@bao-byterover bao-byterover mentioned this pull request Apr 14, 2026
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants