Feat/byterover integration

[hieuntg81]

No description provided.

[hieuntg81]

PR Review: ByteRover Integration

Summary

This PR integrates ByteRover, a long-term project memory CLI tool, into Hermes Agent. It adds:

• byterover_integration/ package (client, recall, onboarding, bridge, prompts) — ~900 lines
• tools/byterover_tool.py — unified brv_command tool with allowlisted subcommands — 442 lines
• run_agent.py — ~160 lines of integration (context injection, onboarding, auto-curate, periodic refresh)
• Docker support (Dockerfile, docker-compose.yml, .dockerignore)
• Tests — 824 lines with good coverage of tool, onboarding, recall modes, bridge
• Minor touches to gateway/run.py, terminal_tool.py, config.py, toolsets.py, model_tools.py, context_compressor.py

3 commits, +2748 / -37 lines, 21 files changed

---

Critical / High

1. API keys visible in process list (Security)

Files: byterover_integration/onboarding.py, byterover_tool.py

API keys (OpenRouter sk-or-*, Anthropic sk-ant-*, ByteRover cloud) are passed as CLI arguments to subprocess.run():

"args": ["providers", "connect", "openrouter", "--api-key", key]

These are visible via ps aux or /proc/*/cmdline on Linux. Should pass keys via environment variable or stdin instead.

2. Shallow copy in flush thread risks data races

File: agent/context_compressor.py

threading.Thread(
    target=brv_flush_on_compress,
    args=(list(messages[self.protect_first_n:]), self.compression_count),
    daemon=True,
).start()

list(...) shallow-copies the list but not the message dicts inside. If compression mutates dict values (e.g., msg["content"] = compressed_content), the flush thread reads corrupted data. Should use copy.deepcopy() or extract text before spawning the thread.

3. run_agent.py is accumulating too much integration logic

File: run_agent.py — +289 / -30 lines

~160 lines of ByteRover logic scattered across _build_system_prompt (30 lines), run_conversation (100+ lines), and _invoke_tool (20 lines). This follows the same pattern as Honcho but compounds the problem. Consider a byterover_integration.hooks module with methods like on_build_prompt(), on_conversation_start(), on_tool_invoked(), on_turn_complete() to keep the agent loop clean.

---

Medium

4. Onboarding prompt mutates persisted user message

File: run_agent.py

user_msg["content"] = _onboard_prefix + user_message

This modifies the user_msg dict before it's added to messages, meaning the [IMPORTANT SYSTEM INSTRUCTION...] prefix gets persisted to session history and replayed on reconnect. Unlike Honcho/brv context injection (which only modifies the API copy), this changes the canonical message.

5. _inject_memory_context lacks prompt injection mitigation

File: run_agent.py

The inline comment acknowledges the trust boundary but does nothing about it. ByteRover context could contain adversarial content if the context tree is shared or if a malicious file is curated. At minimum, wrap the injected context in a delimiter/fence (e.g., <context>...</context>) to reduce injection risk.

6. _brv_recall_mode_snapshot is frozen but _invalidate_system_prompt tries to be dynamic

File: run_agent.py

The snapshot is frozen on first prompt build for cache stability. But _invalidate_system_prompt restores tools from _all_tools and resets _brv_tools_stripped, which then re-evaluates the (stale) snapshot. If the intent is "frozen for session", the invalidation path shouldn't re-evaluate; if the intent is "dynamic", the snapshot should be refreshed.

7. Confusing double-import path

File: tools/byterover_tool.py

The tool module re-exports everything from byterover_integration. Tests import from tools.byterover_tool instead of byterover_integration directly. This creates a two-path import situation where it's unclear which is the canonical source. Tests should import from the integration package directly.

8. _parse_space_list is fragile

File: tools/byterover_tool.py

Only handles tabular output. If brv space list returns JSON (e.g., with --format json), parsing silently returns empty, and onboarding falls through to "No cloud spaces found." Should attempt JSON parsing first, then fall back to line-splitting.

9. Terminal tool guard is over-broad

File: tools/terminal_tool.py

_is_brv_bypass = (
    _stripped.startswith("brv ")
    or "byterover_tool" in _stripped
    or "byterover_integration" in _stripped
    or "run_brv" in _stripped
)

This blocks legitimate commands like grep "byterover_integration" pyproject.toml or cat byterover_integration/README.md. The substring checks are too broad — should only match commands that execute brv or import the module.

---

Low / Nits

• Docker: curl ... | bash - in Dockerfile is a supply chain risk. Consider official Node image as build stage or pinning a hash.
• Unused task_id: brv_command(command, task_id=None) accepts but never uses task_id.
• Lazy import threading: Inconsistent — module-level in client.py, inside function bodies in context_compressor.py and run_agent.py.
• prompt_builder.py re-export: BRV_GUIDANCE is imported into prompt_builder just to be re-imported by run_agent.py. Direct import from byterover_integration.prompts would be cleaner.
• Missing __all__: recall.py and bridge.py lack __all__, making public API unclear.
• Hardcoded magic numbers: _MIN_ENRICH_MESSAGE_LEN = 10, _BRV_ENRICH_TIMEOUT = 8, refresh every 10 turns, etc. — should be configurable or at least consolidated with documented rationale.

---

What's Good

• Comprehensive test coverage — 824 lines covering tool operations, onboarding flow, recall modes, memory bridge, JSON parsing, and edge cases
• Safety allowlist on brv_command subcommands prevents arbitrary CLI execution
• Graceful degradation — everything is wrapped in try/except, brv unavailability is non-fatal
• Parallel prefetch — Honcho and ByteRover run concurrently via ThreadPoolExecutor
• Config version bump — clean migration path for the new byterover config section
• Deterministic onboarding parsing — no LLM in the loop for setup-provider/setup-storage handlers

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Outbound network calls (POST/PUT)

Outbound POST/PUT requests in new code could be data exfiltration. Verify the destination URLs are legitimate.

Matches (first 10):

894:+        with urllib.request.urlopen(req, timeout=10) as resp:
1051:+        with urllib.request.urlopen(req, timeout=15) as resp:
1132:+        with urllib.request.urlopen(req, timeout=10) as resp:

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Outbound network calls (POST/PUT)

Outbound POST/PUT requests in new code could be data exfiltration. Verify the destination URLs are legitimate.

Matches (first 10):

894:+        with urllib.request.urlopen(req, timeout=10) as resp:
1051:+        with urllib.request.urlopen(req, timeout=15) as resp:
1132:+        with urllib.request.urlopen(req, timeout=10) as resp:

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

[github-actions]

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: Outbound network calls (POST/PUT)

Outbound POST/PUT requests in new code could be data exfiltration. Verify the destination URLs are legitimate.

Matches (first 10):

894:+        with urllib.request.urlopen(req, timeout=10) as resp:
1051:+        with urllib.request.urlopen(req, timeout=15) as resp:
1132:+        with urllib.request.urlopen(req, timeout=10) as resp:

⚠️ WARNING: Install hook files modified

These files can execute code during package installation or interpreter startup.

Files:

hermes_cli/setup.py
tests/hermes_cli/test_setup.py

---

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.