Skip to content

fix(swarm): resolve all blocking and medium PR review issues#407

Closed
cuongdo-byterover wants to merge 1 commit intoproj/swarm-memoryfrom
proj/swarm-memory-resolve-comment
Closed

fix(swarm): resolve all blocking and medium PR review issues#407
cuongdo-byterover wants to merge 1 commit intoproj/swarm-memoryfrom
proj/swarm-memory-resolve-comment

Conversation

@cuongdo-byterover
Copy link
Copy Markdown
Collaborator

Summary

  • Problem: The Memory Swarm had no connection to OpenClaw's Memory Wiki — a compiled wiki with structured pages, claims, provenance tracking, and dashboards. Wiki content was invisible to brv swarm query.
  • Why it matters: Memory Wiki contains curated, compiled knowledge that is higher quality than raw notes. Teams using both ByteRover and OpenClaw had to manually search each system separately.
  • What changed: Added memory-wiki as the 7th swarm provider. The adapter reads directly from the wiki vault on disk (no subprocess), uses MiniSearch for fast in-process search (~12ms for 118 pages), supports writes to entities/ and concepts/ directories with proper OpenClaw frontmatter, and applies freshness + kind boosts for structured ranking.
  • What did NOT change (scope boundary): No changes to the OpenClaw wiki plugin itself. No bridge mode changes. No subprocess dependency — the adapter reads files directly. No changes to existing providers or the query pipeline. The adapter follows the same pattern as Obsidian and Local Markdown adapters.

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 ENG-2080

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/core/domain/swarm/types.test.ts (updated: 7 providers)
  • Key scenario(s) covered:
    • Query: 15 diverse queries across 118 wiki pages — 14/15 rank wiki Feat/init #1 (93% precision)
    • Performance: 12ms average per query with 118 pages (in-process MiniSearch)
    • Write: Concept pages written to concepts/ with correct frontmatter, immediately queryable
    • Write routing: Entity → GBrain, note → local-md, general → GBrain, fallback → memory-wiki
    • Cross-provider: 5 providers return results together, wiki ranks highest (weight 0.9)
    • Status: Memory Wiki shows in brv swarm status
    • Enrichment: ByteRover → memory-wiki enrichment visible in --explain

User-visible changes

  • New provider: memory_wiki in .brv/swarm/config.yaml
    memory_wiki:
      enabled: true
      vault_path: ~/.openclaw/wiki/main
      # write_page_type: concept    # default: concept (entity|concept)
      # boost_fresh: true           # default: true
  • Query results show [wiki] source label in whiteBright
  • brv swarm status displays Memory Wiki health
  • brv swarm curate can write to wiki vault (concept/entity pages with OpenClaw frontmatter)
  • RRF weight 0.9 — wiki results rank second only to ByteRover (1.0)
  • Provider excluded from personal queries (wiki content is factual, not personal)

Evidence

  • Trace/log snippets

Status:

✓ ByteRover       context-tree (always on)
✓ Obsidian        /Users/cuong/Documents/MyObsidian
✓ Local .md       1 folder(s)
✓ GBrain          /Users/cuong/workspaces/gbrain
✓ Memory Wiki     OpenClaw wiki

Query with explain (118 pages, 5 providers):

Provider selection: 5 of 5 available
  ✓ byterover    (healthy, selected, 0 results, 14ms)
  ✓ obsidian    (healthy, selected, 5 results, 91ms)
  ✓ local-markdown:project-docs    (healthy, selected, 1 results, 18ms)
  ✓ memory-wiki    (healthy, selected, 2 results, 15ms)
  ✓ gbrain    (healthy, selected, 1 results, 260ms)

Write to wiki:

$ brv swarm curate "Our notification system uses AWS SNS..."
Stored to memory-wiki as concept.swarm.our_notification_system_uses_aws_sns_for_push_notifications

$ cat ~/.openclaw/wiki/main/concepts/our_notification_system_...md
---
pageType: concept
id: concept.swarm.our_notification_system...
status: active
sourceType: swarm-curate
---

Performance (15 queries, 118 pages):

JWT refresh        → wiki: 2 results (15ms) | #1=[wiki]
rate limiting      → wiki: 5 results (15ms) | #1=[wiki]
Kubernetes         → wiki: 3 results (13ms) | #1=[wiki]
chaos testing      → wiki: 1 results (9ms)  | #1=[wiki]
CQRS               → wiki: 4 results (8ms)  | #1=[wiki]

Checklist

  • Tests added or updated and passing (npm test) — 5891 passing
  • Lint passes (npm run lint) — pre-commit hook verified
  • 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: Direct file system access to wiki vault — concurrent writes from OpenClaw and ByteRover could conflict.

    • Mitigation: ByteRover writes to concepts/ and entities/ with a swarm. prefix in the page ID. OpenClaw wiki operations target different page types (sources, syntheses). The .openclaw-wiki/locks/ directory exists for OpenClaw's internal locking — ByteRover does not acquire locks (acceptable for low-frequency writes).
  • Risk: MiniSearch index not invalidated when OpenClaw updates wiki pages externally.

    • Mitigation: Index signature is based on agent-digest.json mtime. When OpenClaw runs wiki compile, the digest is updated, which invalidates the MiniSearch cache on next query.
  • Risk: extractContentSection() may not correctly strip all OpenClaw page templates.

    • Mitigation: Two extraction strategies: (1) look for ```text\n...\n``` code block (standard wiki source page format), (2) fallback to stripping YAML frontmatter. Tested with both imported source pages and manually curated concept pages.

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>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Claude finished @cuongdo-byterover's task in 3m 21s —— View job


PR Review — fix(swarm): resolve all blocking and medium PR review issues

  • Gather context and read changed files
  • Review types.ts and test file
  • Review adapter files (memory-wiki, gbrain, local-markdown, obsidian)
  • Review swarm coordinator, router, config validator
  • Review oclif commands (curate, query)
  • Post inline comments and final summary

Summary

Overall score: 3 / 5

The defensive fixes in this PR (null assertion removal, error type narrowing, execFileSync → execFileAsync, infinite loop guards) are all well-executed. The lazy executor pattern in GBrainAdapter is a clean improvement. However, there are two correctness issues in MemoryWikiAdapter.store() that can produce silently malformed output, and the repo's mandatory TDD policy is not met for the new/changed logic.


Issues

Critical / Correctness

memory-wiki-adapter.ts — YAML title injection (store() ~line 229)
title: "${title}" writes the title unescaped into YAML frontmatter. A title containing a " character (e.g. content starting with # How to use "foo") produces malformed YAML that silently fails to parse when the wiki reads it back.

// current — broken for titles with quotes
`title: "${title}"`,

// fix
`title: ${JSON.stringify(title)}`,

Fix this →

memory-wiki-adapter.ts — Content marker injection (store() ~line 234)
User content is inserted verbatim between and. If the content itself contains the string ``, extractContentSection() will stop early at the first occurrence and the stored content will be silently truncated on read-back.

// fix: sanitize the end marker in user content before insertion
content.replaceAll('', ''),

Fix this →


Medium

Missing unit tests — violates mandatory TDD policy
Per CLAUDE.md, TDD is non-negotiable: tests must be written first and must cover 80%+ of critical paths. This PR adds/changes logic with no corresponding tests:

  • MemoryWikiAdapter.store() — filename collision resolution, YAML frontmatter generation, index invalidation
  • MemoryWikiAdapter.query() — freshness boost, kind boost, extractContentSection() fallback paths
  • MemoryWikiAdapter.ensureIndex() — digest-based cache invalidation
  • resolveUniqueFilename() in local-markdown-adapter.ts — the new MAX_SUFFIX + Date.now() fallback
  • execBrvCurate() — the new MAX_ARG_LENGTH guard
  • classifyQuery() — the removed 'creative' type should have a regression test

Only types.test.ts was updated (count from 6 → 7), which verifies static metadata, not runtime behavior.

memory-wiki-adapter.ts — Dead code guard after ensureIndex() (~line 152)
ensureIndex() always assigns this.index = new MiniSearch(...) unconditionally, so the if (!this.index) return [] check on the next line can never be true. This misleads readers into thinking there's a code path where index stays null after initialization. Either remove it or add a comment explaining why it's a deliberate safety net.

gbrain-adapter.ts L62 — execFileSync inconsistency
resolveGBrainBin() still uses execFileSync to probe for gbrain in PATH. The validator was upgraded to execFileAsync in this same PR. Since resolveGBrainBin() is now called lazily (only on first executor access), it's less impactful, but it still blocks the event loop on first use and is inconsistent with the async pattern adopted everywhere else.


Low / Nitpick

config-validator.ts L6 — import after const (inline comment posted)
import {join} appears after const execFileAsync = ..., violating ESLint import/first. (Inline comment with fix suggestion posted.)

swarm-router.ts L27–31 — Duplicate JSDoc block (inline comment posted)
Two consecutive /** */ blocks for SELECTION_MATRIX — the first is stale. (Inline comment posted.)

types.ts L22 — Silent removal of 'creative' query type (inline comment posted)
No in-code comment explaining the removal. External callers or stored configs using type: 'creative' will break at runtime. (Inline comment posted.)


What's done well

  • GBrainAdapter lazy executor pattern is clean and test-injectable.
  • Null assertion (!) removal in local-markdown and obsidian adapters is correct — the explicit if (!this.index) return [] guard (even if one instance is dead code) is better than a runtime crash.
  • Error type narrowing in curate.ts, query.ts, and swarm-coordinator.ts (instanceof Error + String(error) fallback) follows the CLAUDE.md TypeScript standards correctly.
  • MAX_ARG_LENGTH guard in execBrvCurate is good defensive coding for OS arg limits.
  • memory-wiki correctly excluded from personal query routing, consistent with the intent that wiki content is factual/structured.
  • validateGBrain async conversion removes the last blocking execFileSync in the hot startup path.

Comment thread src/agent/infra/swarm/validation/config-validator.ts
Comment thread src/agent/infra/swarm/swarm-router.ts
Comment thread src/agent/core/domain/swarm/types.ts
@bao-byterover bao-byterover deleted the proj/swarm-memory-resolve-comment branch April 14, 2026 14:19
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.

1 participant