feat(cerebro): add embedded migration and TUI#242
Conversation
Provide embedded SurrealDB migration tooling and an optional in-process TUI for observability without blocking MCP.
Deploying corvus with
|
| Latest commit: |
965be1e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9270bcf1.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-cerebro-tui-migration.corvus-42x.pages.dev |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements embedded SurrealDB storage for Cerebro with migration tooling, adds a feature-gated Terminal UI layer, and extends configuration/redaction infrastructure. It introduces CLI support for legacy export import/validation, event emission for tool-call lifecycle tracking, and optional TUI with sandboxed event bus and data redaction policies. Changes
Sequence Diagram(s)sequenceDiagram
participant MCP as MCP Request
participant Server as CerebroService
participant Tool as CerebroTools
participant Storage as Storage
participant EventBus as EventBus
participant TUI as TUI Task
MCP->>Server: handle_json_rpc(request)
Server->>EventBus: publish(ToolCallEvent::Started)
TUI->>EventBus: recv()
TUI->>TUI: render Started event
Server->>Tool: handle(tool_name, redacted_args)
Tool->>Storage: save/search/delete/etc.
Storage-->>Tool: result
Tool-->>Server: output
Server->>Server: redact_output(output)
Server->>EventBus: publish(ToolCallEvent::Finished)
TUI->>EventBus: recv()
TUI->>TUI: render Finished event
Server-->>MCP: response
sequenceDiagram
participant CLI as CLI User
participant Import as import_legacy_export
participant Legacy as normalize_export
participant Storage as SurrealStorage
participant Report as MigrationReport
CLI->>Import: source, target, options
Import->>Legacy: read_legacy_export(source)
Legacy-->>Import: LegacyExport
Import->>Legacy: normalize_export(legacy)
Legacy-->>Import: NormalizedExport
Import->>Storage: write_batches(normalized)
Storage->>Storage: batch memory/session/prompt
Storage-->>Import: ok
Import->>Report: collection_reports(normalized)
Report-->>Import: checksums & counts
Import-->>CLI: MigrationReport {status: Ok, collections}
sequenceDiagram
participant Main as main.rs
participant Config as CerebroConfig
participant Storage as storage_from_config
participant Service as CerebroService
participant TUI as start_tui_task
participant Shutdown as shutdown signal
Main->>Config: load(CEREBRO_CONFIG)
Config-->>Main: CerebroConfig
Main->>Config: apply_env_overrides()
Config-->>Main: updated config
Main->>Storage: storage_from_config(config).await
Storage-->>Main: Arc<dyn Storage>
Main->>Service: from_config(config).await
Service->>Storage: initialize storage
Storage-->>Service: ready
Service-->>Main: CerebroService
alt TUI enabled
Main->>TUI: start_tui_task(config, storage, event_bus, shutdown_rx)
TUI-->>Main: TuiLaunch::Started(handle)
TUI->>TUI: render loop, subscribe to events
end
Main->>Main: spawn shutdown_signal task
Shutdown-->>Main: CTRL-C detected
Main->>Main: publish shutdown event
TUI->>TUI: cleanup on shutdown
Main->>Service: wait for graceful shutdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The changes span multiple interdependent subsystems (config, storage, migration, TUI, event emission) with significant logic density in surreal storage, migration validation, and event bus mechanics. Heterogeneous edits across configuration validation, async trait implementations, and UI rendering demand separate reasoning per area, though many tests provide good validation anchors. Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-18 to 2026-03-18 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 41
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md`:
- Around line 93-164: The new English-only sections ("Cerebro storage defaults
(embedded)", "Optional TUI (operator-only)", "Migration CLI", and "Operational
notes") create an EN/ES parity gap; update the docs to either provide Spanish
translations for these sections or add a clear "translation pending" note
immediately after each English section header (or at the top of the new block)
indicating that the Spanish version is forthcoming; ensure the note references
the specific section titles so translators/maintainers can find them easily and
keep the headings and keys (e.g., tui.enabled, storage_fallback, migrate
import/validate) intact for parity tracking.
In `@modules/cerebro/src/config.rs`:
- Around line 38-52: The SurrealConfig struct currently serializes the password
field using secret_string_opt which causes the real SecretString to be emitted;
update the serde attributes on the password field so it is not serialized in
plaintext (e.g., stop using a bidirectional serializer and instead use
deserialize-only for secret_string_opt and skip_serializing or provide a
serialize that redacts/omits the value). Target the SurrealConfig.password field
and the secret_string_opt usage so deserialization still accepts a secret but
serialization never returns the real credential.
In `@modules/cerebro/src/main.rs`:
- Around line 27-52: The TUI startup is currently awaited
(start_tui_task(...).await) which can block axum::serve; instead spawn the TUI
startup as a detached background task so the server can enter its accept loop
immediately. Replace the direct .await with a tokio::spawn (or equivalent) that
calls start_tui_task(...) inside an async closure, and move the existing
match/handling logic into that spawned task to log
Ok(TuiLaunch::Started/_Disabled) and Err cases (including
TuiError::FeatureDisabled) without blocking the main thread; keep
validate_no_network_listeners() where it is but do not await the TUI startup on
the main path so axum::serve can start accepting connections promptly.
- Around line 17-20: The code is creating the service from
CerebroConfig::default() and only OR-ing env_flag("CEREBRO_TUI_ENABLED") into
config.tui.enabled, which prevents storage/TUI/auth settings (including
auth_token used by authorize()) from being loaded; replace this flow by fully
loading/parsing the real runtime configuration (from env, CLI and/or config
file) into config before calling CerebroService::from_config, remove the
single-flag OR tweak, and ensure the loaded config contains the auth_token (or
fallback source) so authorize() will accept valid MCP calls.
In `@modules/cerebro/src/migration/legacy.rs`:
- Around line 63-90: normalize_export currently copies LegacyExport.memory into
MemoryRecord.observation without checking the JSON shape; add validation for
each record.observation in normalize_export (e.g., using
serde_json::Value::is_object and Value::get("content") or your expected keys) to
detect malformed data early, and decide on a fail-fast strategy: either change
normalize_export signature from returning NormalizedExport to
Result<NormalizedExport, MigrationError> and return a clear error with the
record id when validation fails, or sanitize/replace invalid observation values
with a well-documented default (and log a warning) before collecting into
memory; reference the functions/types normalize_export, LegacyExport,
MemoryRecord, observation, and NormalizedExport when implementing the checks.
In `@modules/cerebro/src/server.rs`:
- Around line 183-194: The code is publishing raw error.to_string() in the
ToolCallEvent (inside the Err branch that calls self.event_bus.publish with
ToolCallEventKind::Failed), which can leak sensitive request payloads; instead
redact or omit the error before publishing (e.g. call an existing
redaction/sanitization helper like redact_error(error) and set error:
Some(redacted_string) or set error: None / Some("[REDACTED]".to_string()) if no
sanitizer exists) so the published ToolCallEvent does not contain unredacted
tool failure contents.
In `@modules/cerebro/src/storage/mod.rs`:
- Around line 359-372: The Disk fallback branch currently returns
storage_from_mode(config, StorageMode::Disk).await without mapping errors into
the same contextual CerebroError::Storage message used by the RemoteSurreal
branch; update the StorageFallback::Disk arm to call storage_from_mode(config,
StorageMode::Disk).await.map_err(|fallback_error|
CerebroError::Storage(format!("primary storage failed ({error}); fallback failed
({fallback_error})"))) (using the same error variable names as the surrounding
match) so Disk fallback errors are wrapped with consistent fallback context;
ensure you reference the existing storage_from_mode function,
StorageFallback::Disk variant, and CerebroError::Storage constructor when making
this change.
In `@modules/cerebro/src/storage/surreal.rs`:
- Around line 64-68: The current write_batches implementation commits each
collection separately (write_batch_memory, write_batch_sessions,
write_batch_prompts) and uses rollback_records that hard-deletes touched IDs and
swallows errors, which can corrupt pre-existing rows; change the logic to
perform the batch writes inside a single SurrealDB transaction (or an explicit
multi-collection atomic operation) so all collections are committed or none are,
and replace hard-delete rollbacks with restoration of original before-images (or
transaction rollback) while surfacing any rollback errors. Update write_batches
to start a transaction, call the existing write_batch_* helpers (or refactor
their internals) to stage writes within that transaction, commit at the end, and
ensure rollback_records no longer hard-deletes live pre-update rows nor ignores
failures but restores original state and returns errors when rollback fails.
- Around line 52-56: The new_remote constructor for RemoteSurreal currently
always returns CerebroError::NotImplemented while StorageMode::RemoteSurreal and
StorageFallback::RemoteSurreal are publicly selectable; either implement wiring
for the remote SurrealDB client inside pub fn new_remote(_config:
&CerebroConfig) -> Result<Self, CerebroError> (create and return a working
RemoteSurreal instance using cerebро config values and proper error mapping) or
remove/deny the RemoteSurreal option earlier during config parsing (validate
CerebroConfig and return a clear error if StorageMode::RemoteSurreal or
StorageFallback::RemoteSurreal is chosen), and update any code paths that call
new_remote to expect the implemented behavior.
In `@modules/cerebro/src/tools.rs`:
- Around line 43-46: The policy allows an output field for the tool variant
"mem_timeline" but the extractor function extract_safe_output does not handle
"mem_timeline" and returns None, causing policy drift; update
extract_safe_output to add a branch for "mem_timeline" (matching the
allowed_output_fields "items_count") that safely extracts and returns the
items_count (or a sanitized structure) from the tool output, validating types
and boundaries and rejecting/normalizing unexpected values, so the extractor and
the allowed_output_fields for "mem_timeline" remain aligned.
In `@modules/cerebro/src/tui/mod.rs`:
- Around line 187-190: The runtime draw call currently maps any drawing error to
TuiError::InitFailed which is misleading; add a new TuiError variant (e.g.,
RenderFailed) and change the map_err on guard.terminal.draw(|frame|
app.draw(frame)) to map failures to TuiError::RenderFailed(err.to_string());
update any match/handling sites for TuiError to account for the new variant and
keep InitFailed reserved for initialization errors.
- Line 306: Replace the hardcoded VecDeque::with_capacity(200) usages with the
configurable value from TuiConfig (e.g.,
VecDeque::with_capacity(config.event_buffer as usize)) so both the
initialization at events: VecDeque::with_capacity(200) and the other occurrence
use TuiConfig.event_buffer; ensure you pull the config variable (TuiConfig or a
local `config`/`tui_config`) into scope and cast to usize if necessary.
- Around line 348-367: The refresh_storage function currently calls
storage.search("", limit, false, None, None) twice to populate memory_items and
timeline_items, producing identical results; either change the second call used
for timeline_items to use the correct filter/parameters (e.g., a session id or
timeline-specific query instead of the empty string and/or different
boolean/Option args) so timeline_items reflect distinct data, or deduplicate by
running storage.search once and assigning its mapped summaries to both
memory_items and timeline_items if identical results are intended; look for the
refresh_storage, memory_items, timeline_items and the storage.search("", limit,
false, None, None) calls to implement the chosen fix.
In `@modules/cerebro/src/tui/redaction.rs`:
- Around line 55-64: redact_text currently redacts based on substring matches of
sensitive_fields which causes false positives/negatives; update redact_text to
(1) match sensitive field names only as whole words (use case-insensitive
word-boundary matching against sensitive_fields) and (2) additionally detect and
redact common secret patterns (e.g., email addresses, JWTs, base64/hex API keys,
long opaque tokens) using dedicated regex checks before returning REDACTED; keep
REDACTED as the replacement and ensure the checks are performed in redact_text
(and factor reusable regexes into module-level constants) so benign phrases like
"session timeline" aren’t redacted while actual secrets are caught.
In `@modules/cerebro/src/tui/views/live_logs.rs`:
- Around line 21-37: The render loop is cloning strings per frame
(event.timestamp.clone(), event.tool_name.clone(), event.status.clone(),
event.error.clone()); change the status computation to use references (e.g.,
event.status.as_ref().or(event.error.as_ref()).map(|s|
s.as_str()).unwrap_or("-")) and pass &str borrows into Span::raw (use
event.timestamp.as_str() and event.tool_name.as_str()) so no per-frame
allocations occur when building items and ListItem in the map; apply the same
refactor for the similar block at lines 42-47.
In `@modules/cerebro/src/tui/views/memory_explorer.rs`:
- Around line 22-32: The current row construction clones four Strings per row
(timestamp, topic_key, scope, summary); avoid those allocations by passing &str
references into Cell instead of cloning—update the mapping in the iterator that
builds Row::new to use borrowed string slices (e.g., Cell::from(&item.timestamp)
or item.timestamp.as_str()) for each field rather than item.timestamp.clone(),
removing the unnecessary .clone() calls on timestamp, topic_key, scope, and
summary so Cells borrow from item.
In `@modules/cerebro/src/tui/views/mod.rs`:
- Around line 32-53: The Tabs widget and the meta paragraph are rendered into
overlapping regions (both using area and meta_area), so split the available Rect
before rendering: create two distinct regions (e.g., tabs_area and meta_area)
via ratatui::layout::Layout or by computing two Rects from area, allocate the
top region for Tabs (used in
Tabs::new(...).select(...)/frame.render_widget(tabs, tabs_area)) and a separate
one below for the metadata paragraph (used in
Paragraph::new(meta)/frame.render_widget(paragraph, meta_area)); ensure
meta_area.y and height do not overlap tabs_area and use
Constraint::Length/Constraint::Min or saturating arithmetic to guarantee
non-overlap.
In `@modules/cerebro/tests/cli_migration_test.rs`:
- Around line 18-21: The test currently calls .arg(source.to_str().unwrap()) and
.arg(target.to_str().unwrap()), which can panic on non-UTF8 paths; change these
to pass the Path/PathBuf values directly to .arg by using .arg(&source) and
.arg(&target) (and likewise for the second occurrence around lines 34–37), so
that the Command API accepts the &Path without converting to a string.
In `@modules/cerebro/tests/embedded_storage_test.rs`:
- Around line 34-37: The test should verify deletion by attempting to retrieve
(and optionally search for) the deleted item after calling
storage.delete("memory-1", false) instead of relying only on the boolean and
count; update the test to call storage.get("memory-1").await and assert it
returns None (or an Err indicating not found) and optionally call
storage.search(...) to assert the deleted item is not present, keeping the
existing assertions for deleted and storage.count() for completeness.
In `@modules/cerebro/tests/migration_report_test.rs`:
- Around line 65-67: The test fixture loader read_fixture currently uses a
relative PathBuf::from(path) which makes reads cwd-dependent; update
read_fixture to resolve the fixture path against the crate root using
CARGO_MANIFEST_DIR (e.g. join the incoming path with env!("CARGO_MANIFEST_DIR")
or std::env::var("CARGO_MANIFEST_DIR") before calling fs::read_to_string) so
fixtures are loaded deterministically; change the PathBuf construction and keep
the same error expectations in read_fixture.
- Around line 46-63: The test report_status_mismatch_matches_fixture only
asserts two fields (report.status and report.source), which allows unnoticed
regressions; update the test to compare the entire serialized report by calling
report.to_json_value() (or the existing serialization helper used in the first
test) and assert equality against the fixture JSON
(read_fixture("tests/fixtures/reports/validate_report_mismatch.json")), ensuring
the full MigrationReport shape (including collections, target, status, and any
nested fields) matches the fixture exactly.
In `@modules/cerebro/tests/migration_workflow_test.rs`:
- Line 1: Replace brittle string-based status checks in
modules/cerebro/tests/migration_workflow_test.rs with typed enum assertions:
locate where validate_legacy_export/import_legacy_export (and any variables like
result or status) are compared to literal strings and instead assert against the
MigrationStatus enum (e.g., MigrationStatus::Ok) using assert_eq! or pattern
matching; update all occurrences (including the block around lines 20–27) to
reference MigrationStatus variants to get compile-time safety and avoid string
coupling.
In `@modules/cerebro/tests/storage_config_test.rs`:
- Around line 28-39: Test mutates the CEREBRO_TEST_FAIL_STORAGE env var without
synchronization and doesn't guarantee cleanup on panic; create a global
synchronization lock and an RAII guard to set/unset the env var so parallel
tests can't race and cleanup always runs. Add a static/global mutex (e.g.,
once_cell::sync::Lazy<Mutex<()>> ENV_LOCK) and acquire it before setting the env
var in the test, then construct a small guard type (e.g., TestEnvVarGuard) that
sets CEREBRO_TEST_FAIL_STORAGE in its constructor and unsets it in Drop; hold
the lock while the guard exists and while calling storage_from_config().await so
the env mutation + await are serialized and the env var is removed even if the
test panics. Reference symbols: CEREBRO_TEST_FAIL_STORAGE, storage_from_config,
CerebroConfig, InMemoryStorage.
In `@modules/cerebro/tests/tui_event_emission_tests.rs`:
- Around line 17-20: Wrap the unbounded awaits on stream.recv() in a
tokio::time::timeout to avoid hanging tests: replace calls like let started =
stream.recv().await.expect("started event") and let finished =
stream.recv().await.expect("finished event") with timed receives (e.g.
timeout(Duration::from_secs(...), stream.recv()).await) and then unwrap/expect
the timeout result before asserting started.kind and finished.kind against
ToolCallEventKind::Started/Finished; do the same for the other recv uses in this
test so a missing event fails fast with a clear message referencing the stream
recv and the expected ToolCallEventKind.
In `@modules/cerebro/tests/tui_non_blocking_tests.rs`:
- Around line 13-19: The 50ms wall-clock deadline in the tests is too tight and
flakes on CI; update both timeout(...) calls that wrap
service.handle_json_rpc(...) (the instances using Duration::from_millis(50)) to
use a much looser duration (e.g., Duration::from_millis(500) or
Duration::from_secs(1)) or switch to a deterministic test clock, so the tests
catch real stalls instead of scheduler jitter; apply the same change to both
occurrences mentioned in the diff.
In `@modules/cerebro/tests/tui_shutdown_tests.rs`:
- Around line 17-26: The test awaits the background TUI task via
handle.join().await without a timeout, which can hang on regressions; wrap the
join future in a timeout (e.g., tokio::time::timeout with a reasonable Duration)
and assert the timeout did not elapse so the test fails fast on regressions;
apply the same change to both places where start_tui_task/TuiLaunch
handle.join() is awaited so each join is bounded and yields a failing test if
the TUI does not shut down.
- Around line 10-27: The test mutates CEREBRO_TUI_HEADLESS (and
CEREBRO_TUI_TEST_CRASH in the other test) and calls async operations that may
panic or hang, so ensure env mutations are scoped and join() has a timeout: wrap
setting the vars in a short-lived RAII guard (or use a helper that sets the env
and removes it in Drop) so TuiConfig/TuiLaunch/start_tui_task and any early
returns still run cleanup for CEREBRO_TUI_HEADLESS and CEREBRO_TUI_TEST_CRASH;
then wrap handle.join().await with tokio::time::timeout(Duration::from_secs(…))
to fail the test instead of hanging. Reference TuiConfig, start_tui_task,
TuiLaunch, the watch::channel tx/rx setup, and handle.join() when adding the
guard and timeout.
In `@modules/cerebro/tests/tui_toggle_tests.rs`:
- Around line 41-42: The test currently ignores the result of tx.send(true)
which can hide a dropped receiver; change the send call to assert its success
before awaiting the task join (e.g., replace let _ = tx.send(true); with an
assertion such as tx.send(true).expect("toggle send should succeed") or
assert!(tx.send(true).is_ok()) so the test fails if the send fails, then keep
handle.join().await.expect("tui join should succeed"); unchanged.
- Around line 24-63: The async test tui_toggle_enabled_starts_headless mutates
CEREBRO_TUI_HEADLESS without holding ENV_LOCK and it removes the env var instead
of restoring any prior value, which can race with other tests; wrap the body of
tui_toggle_enabled_starts_headless with the same ENV_LOCK used in
tui_validation_rejects_unexpected_listener_env and
tui_validation_allows_no_listener_env, capture the original value of
CEREBRO_TUI_HEADLESS (and any related vars you touch) before setting it, restore
that original value (or remove it if it was absent) in a finally/teardown step
after calling start_tui_task/start_tui_task's returned handle.join(), and apply
the same pattern to any other tests (e.g., in tui_shutdown_tests.rs) that set
env vars; references: ENV_LOCK, tui_toggle_enabled_starts_headless,
CEREBRO_TUI_HEADLESS, start_tui_task, validate_no_network_listeners.
In
`@openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.md`:
- Around line 60-70: The unlabeled fenced code/diagram blocks in design.md
should be annotated with a language (use `text` for ASCII diagrams or `mermaid`
if converting to mermaid); update the blocks that show the call flow (e.g., the
block containing "Cerebro::main", "CerebroConfig::load", and
"storage_from_config") and the other listed ranges (lines showing init
EmbeddedSurrealStore, open/create storage_path, migrations/schema checks, and
fallback policy) by adding the appropriate fence language after the triple
backticks so they no longer trigger MD040; ensure consistency across all similar
blocks in the file.
In
`@openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.md`:
- Around line 11-19: The headings "### In Scope" and "### Out of Scope" violate
MD022; add a single blank line immediately after each subheading so the list
items are separated from the heading (i.e., insert an empty line after the "###
In Scope" and after the "### Out of Scope" lines in proposal.md).
In
`@openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.md`:
- Around line 110-126: The spec section titled "Requirement: TUI Out of Scope
for Phase 1" contradicts the PR which adds TUI config, views, lifecycle wiring,
and tests; either remove the TUI-related changes from this PR or update the spec
delta to reflect that Phase 1 includes TUI work: edit the heading and scenarios
(the "TUI Out of Scope" statement and the two scenarios) to state that TUI
functionality is allowed/optional in Phase 1 or add a new section describing the
introduced TUI config, views, lifecycle wiring, and tests, and ensure the spec
language (requirements and scenarios) exactly matches the PR changes so docs
remain authoritative.
In
`@openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.md`:
- Around line 57-63: The fenced ASCII flow block is unlabeled and triggers
markdownlint MD040; update the fence marker for that block from ``` to ```text
so it becomes a labeled code fence (e.g., change the block that begins with the
ASCII diagram containing "Agent ── MCP tools/call ──→ Cerebro MCP server ──→
SurrealDB (embedded or remote)" to use ```text) to silence the linter and keep
the archived doc lint-clean.
In
`@openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md`:
- Around line 1-24: Change the top-level "## Verification Report" to a proper H1
(prepend a single "#") and normalize blank lines so there is exactly one blank
line before and after each heading and before/after the markdown table;
specifically update the "## Verification Report" (make it "# Verification
Report"), the "### Completeness" and "### Build & Tests Execution" headings to
have one blank line above and below, and ensure the table has a single blank
line separating it from surrounding headings/text (repeat same normalization for
the other occurrences referenced at 49-50, 61-62, 86-87).
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.md`:
- Around line 164-167: Update the "Open Questions" section to document the
resolved decision: state that the terminal UI crate selected is ratatui v0.28.0
with crossterm v0.28.0 as the backend and that both dependencies were added to
Cargo.toml; mention the crates by name (ratatui, crossterm) and their versions
so readers know which libraries (and versions) the implementation uses and where
to find them in the project.
- Around line 52-80: Add a language specifier (e.g., ```text or ```plaintext) to
each ASCII-art code block to satisfy static analysis: the block starting with
"Client ── tools/call ──→ CerebroService", the "TUI task lifecycle" block
beginning "Cerebro main ── config flag ──→ start_tui_task()", and the "View
query and refresh loop" block beginning "TUI view ── periodic query ──→
Storage"; update the opening fences for those three blocks to include the chosen
specifier so the diagrams remain readable and the linter warning is suppressed.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.md`:
- Around line 8-14: The Markdown violates MD022/MD058 by lacking blank lines
around headings and tables; edit verify-report.md (look for the "###
Completeness" heading and its following table and the other flagged blocks
around lines showing the other sections) and ensure there is exactly one empty
line before and after each heading and before and after each table block (add a
blank line between the previous paragraph and "### Completeness", and a blank
line after the table, and apply the same fix to the other flagged blocks such as
the sections referenced at 50-52, 63-65, and 80-81).
- Line 1: The markdown header on the first line uses a level-2 header ("##
Verification Report") which triggers MD041; change that exact string to a
level-1 header ("# Verification Report") so the file begins with an H1 and
satisfies the linter (reference strings: "## Verification Report" -> "#
Verification Report", rule MD041).
In `@openspec/specs/cerebro/prompt_template.md`:
- Around line 1-36: The document "Cerebro Agent Prompt Template" currently only
contains English content; add ES parity by providing a Spanish translation of
the user-facing sections (at minimum the "Copy-Paste Template" and "Usage
Notes") or, if translation cannot be completed now, insert an explicit note
labeled "ES pending" near the top (e.g., under the main header) that states
Spanish translation is pending and include a tracking ID or link to the
issue/board; update the file sections "Copy-Paste Template" and "Usage Notes"
(or add a new "ES / Español" section) and ensure the note references where to
find or track the translation work.
In `@openspec/specs/cerebro/spec.md`:
- Around line 28-34: The fenced ASCII architecture block starting with "Agent
Runtime ── MCP tools/call ──→ Cerebro MCP Server ──→ SurrealDB (embedded or
remote)" is missing a language tag and is flagged by markdownlint-cli2; update
the opening fence to include a language (e.g., change ``` to ```text or
```plaintext) so the block is lint-clean, or alternatively convert the diagram
to a mermaid graph and mark the fence as ```mermaid if you want rendered
diagrams; ensure you update the same fenced block in spec.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 769aae20-e9b0-462c-a5c0-c4549fd61cb1
⛔ Files ignored due to path filters (2)
clients/agent-runtime/Cargo.lockis excluded by!**/*.lockmodules/cerebro/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (59)
.agents/skills/conventional-commits/SKILL.md.gitignoreclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdmodules/cerebro/Cargo.tomlmodules/cerebro/src/config.rsmodules/cerebro/src/lib.rsmodules/cerebro/src/main.rsmodules/cerebro/src/migration/checksum.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/src/migration/report.rsmodules/cerebro/src/server.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/event_bus.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/tui/views/dashboard.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/src/tui/views/memory_explorer.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/src/tui/views/session_timeline.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/tests/fixtures/legacy/legacy_export.jsonmodules/cerebro/tests/fixtures/reports/import_report_ok.jsonmodules/cerebro/tests/fixtures/reports/validate_report_mismatch.jsonmodules/cerebro/tests/helpers/mod.rsmodules/cerebro/tests/mcp_auth_policy.rsmodules/cerebro/tests/mcp_tools_contract.rsmodules/cerebro/tests/migration_checksum_test.rsmodules/cerebro/tests/migration_integration_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_event_bus_tests.rsmodules/cerebro/tests/tui_event_emission_tests.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/tui_redaction_tests.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/tui_toggle_tests.rsopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/proposal.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/tasks.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/proposal.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/tasks.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdopenspec/specs/cerebro/prompt_template.mdopenspec/specs/cerebro/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/proposal.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/proposal.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/tasks.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdopenspec/specs/cerebro/prompt_template.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/tasks.mdopenspec/specs/cerebro/spec.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/src/tui/views/live_logs.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/proposal.mdmodules/cerebro/src/migration/report.rsmodules/cerebro/src/tui/views/dashboard.rsmodules/cerebro/tests/migration_integration_test.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/proposal.mdmodules/cerebro/tests/fixtures/reports/validate_report_mismatch.jsonmodules/cerebro/src/migration/checksum.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/lib.rsmodules/cerebro/tests/helpers/mod.rsmodules/cerebro/tests/tui_shutdown_tests.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/tasks.mdmodules/cerebro/tests/tui_non_blocking_tests.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/mcp_auth_policy.rsopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdmodules/cerebro/src/tools.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/specs/cerebro/spec.mdmodules/cerebro/tests/tui_redaction_tests.rsmodules/cerebro/src/server.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/tests/fixtures/legacy/legacy_export.jsonmodules/cerebro/src/tui/views/session_timeline.rsmodules/cerebro/src/tui/views/memory_explorer.rsmodules/cerebro/tests/tui_event_emission_tests.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdmodules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/tests/mcp_tools_contract.rsopenspec/specs/cerebro/prompt_template.mdmodules/cerebro/Cargo.tomlmodules/cerebro/src/migration/mod.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_event_bus_tests.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/config.rsmodules/cerebro/src/tui/mod.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/src/tui/event_bus.rsmodules/cerebro/src/tui/views/mod.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/tasks.mdopenspec/specs/cerebro/spec.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdmodules/cerebro/src/storage/surreal.rsmodules/cerebro/tests/migration_checksum_test.rsmodules/cerebro/tests/fixtures/reports/import_report_ok.jsonmodules/cerebro/src/main.rsmodules/cerebro/src/tui/redaction.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
modules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/src/migration/report.rsmodules/cerebro/src/tui/views/dashboard.rsmodules/cerebro/tests/migration_integration_test.rsmodules/cerebro/src/migration/checksum.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/lib.rsmodules/cerebro/tests/helpers/mod.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/mcp_auth_policy.rsmodules/cerebro/src/tools.rsmodules/cerebro/tests/tui_redaction_tests.rsmodules/cerebro/src/server.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/tui/views/session_timeline.rsmodules/cerebro/src/tui/views/memory_explorer.rsmodules/cerebro/tests/tui_event_emission_tests.rsmodules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/tests/mcp_tools_contract.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_event_bus_tests.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/config.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/src/tui/event_bus.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/tests/migration_checksum_test.rsmodules/cerebro/src/main.rsmodules/cerebro/src/tui/redaction.rs
🧠 Learnings (8)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
modules/cerebro/tests/cli_migration_test.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/src/server.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
modules/cerebro/src/tools.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
modules/cerebro/tests/mcp_tools_contract.rs
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Maintain comprehensive agent metadata including name, description, purpose, and capabilities
Applied to files:
openspec/specs/cerebro/prompt_template.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
modules/cerebro/Cargo.toml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
modules/cerebro/Cargo.toml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
modules/cerebro/src/tui/redaction.rs
🪛 LanguageTool
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.md
[style] ~19-~19: The double modal “requires embedded” is nonstandard (only accepted in certain dialects). Consider “to be embedded”.
Context: ...s. Rationale: The proposal requires embedded SurrealDB as the default for new deploy...
(NEEDS_FIXED)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/proposal.md
[warning] 13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/proposal.md
[warning] 57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 50-50: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 61-61: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 62-62: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.md
[warning] 60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 50-50: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 51-51: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 64-64: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 80-80: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.md
[warning] 57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.md
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 64-64: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/specs/cerebro/spec.md
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (32)
.gitignore (1)
131-131: Good safeguard for local embedded DB artifacts.This ignore rule is appropriately scoped and helps prevent accidental commits of runtime storage files.
.agents/skills/conventional-commits/SKILL.md (1)
8-8: LGTM: Metadata attribution update.Simple authorship metadata update with no functional impact.
modules/cerebro/src/tools.rs (1)
16-63: Deny-by-default fallback is implemented correctly.Returning empty allowlists for unknown tools is a solid default for redaction safety.
modules/cerebro/tests/tui_toggle_tests.rs (1)
11-22: Disabled-path test is clean and deterministicGood coverage of the no-op launch branch with explicit
TuiLaunch::Disabledmatching.modules/cerebro/tests/mcp_tools_contract.rs (1)
27-27: Token construction update is safe and consistent.The
SecretStringinitialization change is non-breaking for these tests and keeps auth fixture construction aligned across this suite.Also applies to: 54-54, 134-134, 205-205
modules/cerebro/tests/mcp_auth_policy.rs (1)
9-9: Auth policy tests remain behaviorally unchanged.This token-construction adjustment is safe and does not alter the policy checks being validated.
Also applies to: 33-33, 58-58, 82-82
modules/cerebro/tests/fixtures/legacy/legacy_export.json (1)
1-38: Fixture coverage is well-structured for migration tests.The dataset includes useful state variation (active/deleted memory + related session/prompt) and looks fit for normalization/import validation scenarios.
openspec/changes/2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.md (1)
1-45: Task breakdown is clear and implementation-oriented.The phased checklist and explicit file mapping improve delivery traceability for this migration effort.
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/tasks.md (1)
1-45: Archived task plan is complete and coherent.This is a solid implementation ledger for the optional TUI surface and associated verification scope.
modules/cerebro/src/migration/checksum.rs (1)
3-52: Deterministic canonicalization and checksum flow looks correct.Key ordering, recursive serialization, and digest formatting are coherent for stable migration validation output.
modules/cerebro/tests/fixtures/reports/validate_report_mismatch.json (1)
1-10: Fixture shape and mismatch status are coherent with validation-report testing.modules/cerebro/src/lib.rs (1)
7-7: Thetuimodule declaration can safely remain unconditional.The exposed submodules (
redaction,event_bus) are feature-independent and contain noratatui/crosstermusage. All TUI-specific code is properly gated with#[cfg(feature = "tui")]at the module internals level—no-feature builds will compile without issues.> Likely an incorrect or invalid review comment.modules/cerebro/Cargo.toml (1)
23-23: Remove unusedclapdependency or provide justification.
clapis listed in Cargo.toml (line 23) but has zero usage in the codebase. The binary uses environment variables for configuration instead of CLI argument parsing. Either remove the dependency or document the intent if it is planned for future use.> Likely an incorrect or invalid review comment.modules/cerebro/tests/fixtures/reports/import_report_ok.json (1)
1-10: Fixture shape is consistent with expected OK migration report contract.
This is a solid positive-path fixture for import/validate report assertions.modules/cerebro/tests/tui_event_bus_tests.rs (1)
17-28: Good coverage for lagging-subscriber drop accounting.
The test captures the intended bounded-buffer behavior without over-constraining timing.modules/cerebro/tests/migration_legacy_test.rs (1)
4-24: Tests cover critical legacy parse/normalize expectations well.
Count checks plus normalized ID assertions provide useful guardrails for migration behavior.openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/tasks.md (1)
1-26: Checklist structure and phase breakdown look clean and actionable.
No correctness or alignment concerns in this tasks document.modules/cerebro/tests/helpers/mod.rs (1)
5-31: Useful and cohesive test helpers.
These helpers reduce duplication and keep auth/request setup consistent across tests.modules/cerebro/tests/migration_checksum_test.rs (1)
3-32: Good coverage of canonical checksum invariants.These tests correctly lock down the critical ordering behavior for objects vs arrays and validate checksum format.
modules/cerebro/tests/tui_redaction_tests.rs (1)
5-43: Solid security-focused redaction coverage.These tests capture both explicit sensitive-key redaction and default-deny behavior for unknown fields.
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/specs/cerebro/spec.md (1)
1-137: Requirements and scenarios are clear and testable.The spec cleanly defines optionality, safety constraints, and expected runtime behavior for TUI without weakening MCP guarantees.
openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/proposal.md (1)
1-79: Proposal aligns well with PR objectives.The document correctly scopes embedded SurrealDB to the Cerebro service (not agent-runtime) and stages TUI as optional. Conflicts are explicitly documented with clear mitigation and rollback guidance.
Minor: Static analysis flags missing blank lines before "### In Scope" (line 13), "### Out of Scope" (line 24), and "### Conflicts / Deltas to Resolve" (line 36). Adding blank lines before these headings improves markdown rendering consistency.
modules/cerebro/src/migration/report.rs (1)
1-57: Clean, idiomatic implementation.Good use of
BTreeMapfor deterministic ordering andunwrap_or_elsefor graceful fallback on serialization failure. The#[serde(rename_all = "snake_case")]correctly aligns with the manualas_str()implementation.openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/specs/cerebro/spec.md (1)
1-153: Spec aligns with implementation.The 13-tool inventory, optional TUI views (dashboard, memory explorer, session timeline, live logs), and SurrealDB scoping to Cerebro-only match the code changes in this PR. Data hygiene scenarios covering soft-delete, deduplication, and topic-key upserts are well-specified.
modules/cerebro/src/migration/legacy.rs (1)
48-61: Blocking I/O is acceptable for migration context.The
std::fs::read_to_stringis synchronous, which is fine for a migration CLI utility. Error context is clear with path display included.modules/cerebro/src/storage/mod.rs (2)
61-61:as_anyaddition is appropriate for runtime downcasting.The
as_anytrait method enables type-checked downcasting when needed (e.g., test assertions, specialized behavior). Implementation is straightforward.
390-395: No action needed—new_remoteis intentionally synchronous.
SurrealStorage::new_remoteis a stub that returnsCerebroError::NotImplemented, so it requires no async I/O. The synchronous signature is correct and consistent with the current implementation.modules/cerebro/src/tui/mod.rs (2)
43-61: Good security check for network listener prevention.
validate_no_network_listenerscorrectly blocks environment variables that could expose the TUI over network, maintaining the in-process-only security posture.
79-95: Panic isolation is properly implemented.
catch_unwindwithAssertUnwindSafeensures TUI panics don't crash the MCP server. Logging the panic/error before exiting is appropriate.modules/cerebro/src/tui/event_bus.rs (2)
27-57: Event bus design is sound.Bounded broadcast with explicit drop accounting favors MCP throughput over perfect observability, as documented.
buffer.max(1)prevents zero-capacity edge case. Relaxed ordering on the drop counter is appropriate for non-critical observability metrics.
64-91: Lag handling is correct.Both
recvandtry_recvproperly accumulate lagged counts before continuing. The sharedAtomicU64counter allows aggregate drop tracking across all subscribers.openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.md (1)
1-148: Design document aligns well with implementation.Architecture decisions (in-process TUI, broadcast event bus, redaction at emission) match the code. Failure modes and testing strategy are comprehensive and reflected in the implemented panic isolation, drop accounting, and test fixtures.
Load config files, validate embedded SurrealDB loopback/auth, report fallback mode, and expand migration tests.
# Conflicts: # clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
186-194:⚠️ Potential issue | 🟡 MinorDocumentation gap: New features not reflected in README.
The PR introduces significant new capabilities (embedded SurrealDB deployment, migration tooling, optional TUI, storage fallback policies) but the README contains no updates describing these features or how to use them. Users reading the README won't discover the new embedded mode, TUI options, or migration commands.
Consider adding a brief section covering:
- Embedded vs. remote SurrealDB deployment modes
- Migration tooling usage (referencing the detailed migration guide)
- TUI enablement and configuration
As per coding guidelines: "Verify technical accuracy and that docs stay aligned with code changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 186 - 194, Add a concise README subsection that documents the new embedded SurrealDB mode, how to switch between embedded and remote SurrealDB (mention the embedded-surreal/remote-surreal modes or flag like --embedded-surreal), how to run the migration tooling (point readers to the migration guide and reference the migration command name used in the repo), how to enable the optional TUI (mention the --enable-tui flag or TUI config option), and a short note about storage fallback policies (reference the storage.fallback-policy/config key); include links to the detailed migration guide and MCP schema docs already in the repo so users can follow up for full instructions.
♻️ Duplicate comments (3)
modules/cerebro/tests/tui_shutdown_tests.rs (1)
33-60:⚠️ Potential issue | 🟠 MajorSerialize env mutations to avoid inter-test races.
Line 34 and Line 58 set process-wide env vars without a shared lock. These async tests can run concurrently and leak
CEREBRO_TUI_TEST_CRASH=1into the other test path, causing flakes.🔧 Suggested fix
+use std::sync::Mutex; use tokio::sync::watch; use tokio::time::{timeout, Duration}; +static ENV_LOCK: Mutex<()> = Mutex::new(()); + #[tokio::test] async fn tui_exits_on_shutdown_signal() { + let _env_lock = ENV_LOCK.lock().expect("env lock"); let _headless = EnvVarGuard::set("CEREBRO_TUI_HEADLESS", "1"); @@ #[tokio::test] async fn tui_crash_isolated_from_caller() { + let _env_lock = ENV_LOCK.lock().expect("env lock"); let _headless = EnvVarGuard::set("CEREBRO_TUI_HEADLESS", "1"); let _crash = EnvVarGuard::set("CEREBRO_TUI_TEST_CRASH", "1");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/tui_shutdown_tests.rs` around lines 33 - 60, Both tests mutate process-wide env vars via EnvVarGuard::set (in tui_exits_on_shutdown_signal and tui_crash_isolated_from_caller) and can race when run concurrently; serialize these mutations by adding a shared test lock and acquiring it before calling EnvVarGuard::set. Concretely, introduce a static test mutex (e.g., TEST_ENV_LOCK using once_cell::sync::Lazy<Mutex<()>> or similar) and lock it at the start of each test before creating the EnvVarGuard, holding the lock until the guard is dropped so env changes cannot leak between tests; update both tests to acquire that mutex around their EnvVarGuard::set calls.modules/cerebro/tests/tui_non_blocking_tests.rs (1)
51-53:⚠️ Potential issue | 🟠 MajorMake shutdown verification explicit and non-blocking.
Line 51 drops the
sendresult, and Line 52 performs an unboundedjoin. That can hide shutdown-signal failures and hang CI.🔧 Suggested fix
- let _ = shutdown_tx.send(true); - handle.join().await.expect("tui join"); + shutdown_tx.send(true).expect("shutdown send should succeed"); + timeout(Duration::from_secs(1), handle.join()) + .await + .expect("tui join timeout") + .expect("tui join");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/tui_non_blocking_tests.rs` around lines 51 - 53, The test currently drops the result of shutdown_tx.send(true) and awaits handle.join() unbounded, which can hide send failures and hang CI; change to assert the send succeeded (e.g., check shutdown_tx.send(true) returns Ok or use expect/unwrap) and wrap the join await with a timeout (use tokio::time::timeout with a short Duration and then unwrap/expect both the timeout result and the join result) so failures and hangs fail the test fast; update imports as needed and keep the existing std::env::remove_var("CEREBRO_TUI_HEADLESS") cleanup.modules/cerebro/src/tui/redaction.rs (1)
19-20:⚠️ Potential issue | 🟡 MinorBroad TOKEN_RE pattern may cause false positives.
TOKEN_REmatches any alphanumeric string of 24+ characters, which includes UUIDs without hyphens, base58 identifiers, and other legitimate non-secret data. This could over-redact user content.Consider tightening the pattern or adding context-aware filtering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tui/redaction.rs` around lines 19 - 20, TOKEN_RE currently matches any alphanumeric sequence >=24 chars which causes false positives; update the redaction logic by replacing or refining TOKEN_RE in redaction.rs to be more specific (for example require known token prefixes/suffixes, include context words like "token", "auth", "secret" in lookbehind/lookahead, or enforce character classes/entropy constraints) and/or add an additional context-aware check after a match (e.g., skip if it matches a UUID-without-hyphens pattern or other whitelisted formats) so only true secrets are redacted; ensure changes reference the TOKEN_RE symbol and the redaction flow that consumes its matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lychee.toml`:
- Around line 16-17: The exclusion patterns currently use
'https://docs\.github\.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets'
and 'https://packages\.ubuntu\.com/?' which are too permissive; update these
patterns to add anchors and proper optional query handling (e.g., append ^ and $
and allow optional query string via (?:\?.*)?$) so they only match the exact
intended URLs: modify the two patterns to something like
'^https://docs\.github\.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets(?:\?.*)?$'
and '^https://packages\.ubuntu\.com/?(?:\?.*)?$' (apply equivalent anchored
regex) to prevent unintended URL matches.
In `@modules/cerebro/src/migration/legacy.rs`:
- Around line 99-113: In validate_observation, replace the negated is_some()
check with the idiomatic is_none(): change the condition that currently uses
!observation.get("content").is_some() to observation.get("content").is_none() so
the warning branch when the "content" field is missing remains the same but uses
the clearer, more idiomatic check in the validate_observation function
(reference symbols: validate_observation, observation.get("content"),
memory_id).
In `@modules/cerebro/src/migration/mod.rs`:
- Around line 98-110: The cache check/insert around STORAGE_CACHE.get_or_init
and SurrealStorage::new_embedded has a TOCTOU race: between locking to read the
HashMap by cache_key and later inserting the new value another thread may create
the same storage; to fix, acquire and use the HashMap::entry API (or hold the
Mutex lock) so you check for an existing entry and only call
SurrealStorage::new_embedded when absent, ensuring a single initialization per
cache_key; modify the code paths that reference STORAGE_CACHE, cache_key, and
SurrealStorage::new_embedded to perform the lookup/insert atomically via entry
or by keeping the lock while creating or storing a placeholder/result.
In `@modules/cerebro/src/storage/surreal.rs`:
- Around line 341-386: The search and count implementations currently load all
records via self.db.select("memory") and filter/count in Rust (functions search
and count); change them to run SurrealQL server-side queries using parameterized
WHERE clauses and COUNT() to avoid pulling the whole table: for search build a
SELECT ... FROM memory WHERE ... (respect include_deleted, scope, topic_key, and
a case-insensitive CONTAINS on summary or topic_key), ORDER BY timestamp DESC
LIMIT $limit and execute it via self.db.query or the appropriate client call,
mapping results to Vec<MemoryRecord> and preserving CerebroError mapping; for
count run a SELECT count() FROM memory with the same WHERE predicates (or GROUP
ALL) and return the numeric result instead of records.len().
In `@modules/cerebro/src/tools.rs`:
- Around line 273-280: The mem_suggest_topic_key branch currently exposes the
user-provided "seed" in logs; update the handler for "mem_suggest_topic_key"
(the block that deserializes ToolInput<MemSuggestTopicKeyRequest> from payload
and builds the json response) and the allowed_arg_fields allowlist to remove
"seed" so only "scope" is kept; specifically, stop including input.input.seed in
the returned JSON (only include input.input.scope) and remove "seed" from the
allowed_arg_fields entry for "mem_suggest_topic_key" to prevent logging
user-provided text.
- Around line 233-293: The function extract_safe_args is cloning payload in
every match arm (payload.clone()) which is unnecessary; change the signature to
take payload by value (fn extract_safe_args(&self, tool: &str, payload: Value)
-> Option<Value>) and remove all payload.clone() calls, using
serde_json::from_value(payload) in each branch instead, and update all call
sites to pass ownership (or if call-site changes are infeasible, clone once at
the start: let payload = payload.clone(); and use that single owned Value for
all serde_json::from_value calls). Ensure references to extract_safe_args,
payload.clone(), and serde_json::from_value are updated accordingly.
In `@modules/cerebro/src/tui/mod.rs`:
- Around line 203-213: The runtime event errors from crossterm::event::poll and
crossterm::event::read are incorrectly mapped to TuiError::InitFailed; add a new
error variant (e.g., TuiError::EventError) and change the map_err calls around
poll and read to produce TuiError::EventError(err.to_string()) so runtime
polling/reading failures are reported correctly; leave TuiError::InitFailed for
actual initialization failures and ensure the new variant is used wherever event
loop I/O errors occur (search for crossterm::event::poll,
crossterm::event::read, and app.on_key to update usages).
In `@modules/cerebro/src/tui/redaction.rs`:
- Around line 87-115: The recursion drops the allowlist (causing all nested
fields to be redacted): update redact_value so recursive calls propagate the
incoming allowlist instead of passing None; specifically, in the Value::Object
branch call self.redact_value(value, allowlist) when inserting non-redacted
nested values and in the Value::Array branch map to self.redact_value(value,
allowlist). Keep the existing normalization/contains check against
self.sensitive_fields and REDACTED behavior unchanged.
In `@modules/cerebro/src/tui/views/mod.rs`:
- Around line 32-35: The tabs area is too short because Constraint::Length(2)
leaves no content inside the bordered block; in the Layout::default() ->
.direction(Direction::Vertical) -> .constraints([...]) call that assigns to
chunks, change the tabs constraint from Constraint::Length(2) to at least
Constraint::Length(3) so the bordered tabs region has space for borders plus the
tab labels.
In `@modules/cerebro/tests/cli_migration_test.rs`:
- Around line 63-65: Ensure the test actually exercises the mutation by
asserting the "memory" array exists and is non-empty before calling array.pop();
replace the optional silent-path using
json.get_mut("memory").and_then(Value::as_array_mut) and array.pop() with an
explicit precondition assertion (e.g., assert that
json.get("memory").and_then(Value::as_array).map(|a|
!a.is_empty()).unwrap_or(false)) so the test fails if "memory" is missing or
empty and thus guarantees the intended mismatch mutation occurs.
In `@modules/cerebro/tests/migration_legacy_test.rs`:
- Around line 5-7: The test parses_legacy_export_fixture uses a relative PathBuf
("tests/fixtures/legacy/legacy_export.json") which is cwd-dependent; change it
to build the fixture path using the crate root constant like in other tests (use
env!("CARGO_MANIFEST_DIR") combined with Path/PathBuf) before calling
read_legacy_export so the fixture loads deterministically; update the PathBuf
construction in parses_legacy_export_fixture to use env!("CARGO_MANIFEST_DIR")
joined with "tests/fixtures/legacy/legacy_export.json".
In `@modules/cerebro/tests/storage_config_test.rs`:
- Around line 137-148: The test helper currently ignores a poisoned mutex in
BufferGuard::write by using if let Ok(self.0.lock()), which can hide prior
panics; change the behavior so a poisoned lock fails the test: either call
self.0.lock().expect("BufferGuard mutex poisoned") to panic immediately or
detect Err(_) and return an Err(std::io::Error::new(std::io::ErrorKind::Other,
"mutex poisoned")) from write so the test fails; update the implementation in
BufferGuard::write to handle the Result from self.0.lock() explicitly instead of
silently skipping on Err.
- Around line 150-169: In fallback_reports_active_mode, acquire the global
ENV_LOCK before mutating the environment variable to avoid races: lock ENV_LOCK
(e.g., let _env_lock = ENV_LOCK.lock().unwrap()) prior to calling
EnvVarGuard::set("CEREBRO_TEST_FAIL_STORAGE", "1") and hold it through the call
to storage_from_config (and while reading the buffer) so the env change is
protected; ensure the lock is dropped at the end of the test so other tests can
proceed. Use the existing symbols fallback_reports_active_mode, EnvVarGuard,
ENV_LOCK, and storage_from_config to locate where to add the lock.
In `@modules/cerebro/tests/tui_toggle_tests.rs`:
- Around line 71-73: The test currently awaits handle.join() without a timeout
which can hang; wrap the join in a tokio::time::timeout (e.g., 5s) and assert
the timeout did not occur, then unwrap/expect the join result so the test fails
fast if shutdown regresses—update the test to call
tokio::time::timeout(Duration::from_secs(5), handle.join()).await and assert
it's Ok before unwrapping the JoinHandle result after tx.send(true) and
handle.join() calls.
In
`@openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md`:
- Around line 8-90: Change all top-level section headings that currently use
"###" (e.g., "### Completeness", "### Build & Tests Execution", "### Spec
Compliance Matrix", "### Correctness (Static — Structural Evidence)", "###
Coherence (Design)", "### Issues Found", "### Verdict") to use "##" so they sit
directly under the H1, and ensure there is a blank line immediately after the
final Verdict heading (now "## Verdict") to satisfy markdownlint rules
MD001/MD022.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md`:
- Around line 21-29: The fenced code blocks in the verify-report markdown are
missing language specifiers and surrounding blank lines; update each fenced
block (the ones containing the test command/results output) to use a language
tag such as ```text or ```shell and ensure there is a blank line immediately
before the opening ``` and a blank line immediately after the closing ``` to
satisfy MD031/MD040.
- Around line 73-81: The markdown table under the heading that starts with
"Requirement | Status | Notes" is missing trailing pipe characters on the rows
for entries like "Embedded storage default", "No silent fallback on init
failure", "Migration CLI subcommands", "File-based legacy export import",
"Validation uses counts + checksums", and "Migration report schema"—add a
trailing '|' at the end of each table row to satisfy MD055; also insert a blank
line immediately after the heading above the table so the heading is separated
from the table as required.
- Line 1: Replace the second-level Markdown heading "## Verification Report"
with a top-level heading by changing the text to "# Verification Report" so the
document uses H1 for the first heading (replace the exact string "##
Verification Report" with "# Verification Report").
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.md`:
- Around line 83-84: The "### Verdict" heading in verify-report.md is missing a
blank line after it; insert a single empty line between the "### Verdict"
heading and the following "PASS" text so the heading is separated from its
content (i.e., change "### Verdict\nPASS" to "### Verdict\n\nPASS") to satisfy
MD022.
- Line 8: The document uses H3 headings that skip from H1 to H3; update the
heading levels so they increment by one (change "### Completeness" to "##
Completeness") and apply the same adjustment to the other affected headings
referenced in the review (e.g., the headings with text at the suggested-fix
pattern positions), ensuring all headings follow H1 -> H2 -> H3 ordering
consistently throughout the file.
In `@openspec/specs/cerebro/spec.md`:
- Around line 524-541: The "Requirement: TUI Out of Scope for Phase 1" section
conflicts with the existing TUI requirements elsewhere; either mark the detailed
TUI requirements as "future phase" or remove the out-of-scope statement. Update
the spec so the "TUI Out of Scope for Phase 1" heading explicitly references
that TUI requirements (the "TUI requirements" sections) apply only when TUI is
enabled in a future phase (e.g., add "These TUI requirements apply in a future
phase when the TUI is enabled") or delete the out-of-scope block if TUI is
actually intended to be in-scope for this change, and ensure cross-references
between the "TUI Out of Scope for Phase 1" heading and the TUI requirements
sections are consistent.
In `@PRD-Corvus.md`:
- Around line 3-37: Several Markdown headings in PRD-Corvus.md (e.g., "## 1.
Introduction and Objectives", "### Objectives", "## 2. Key Features", "## 3.
Architecture Overview", "## 4. Security and Performance Principles", "## 5.
Development Workflow", and "## 6. Integration and Testing") lack the required
blank line after the heading; update the file so each heading line is followed
by a single blank line before the next paragraph or list to satisfy MD022
linting. Ensure you add the blank line immediately after each heading string
listed above (and any other headings in the document) without changing heading
text or content.
In `@PRD-English.md`:
- Around line 3-39: The markdown headings (e.g., "## 1. Introduction and
Objectives", "### Objectives", "## 2. Key Features", "## 3. Architecture
Overview", "## 4. Security and Performance Principles", "## 5. Development
Workflow", "## 6. Integration and Testing") are missing a blank line below them
which triggers MD022; add a single blank line immediately after each of these
headings so there is an empty line between the heading line and the following
paragraph or list item throughout the document.
In `@README.md`:
- Line 194: The current level-2 heading that only contains the badge `## [](https://deepwiki.com/dallay/corvus)`
breaks markdown semantics; fix it by either removing the leading "## " to make
the badge a standalone line (`[](https://deepwiki.com/dallay/corvus)`)
or replace that line with a descriptive heading `## Ask DeepWiki` and move the
badge to the next line so the heading contains meaningful text and the badge
remains visible.
---
Outside diff comments:
In `@README.md`:
- Around line 186-194: Add a concise README subsection that documents the new
embedded SurrealDB mode, how to switch between embedded and remote SurrealDB
(mention the embedded-surreal/remote-surreal modes or flag like
--embedded-surreal), how to run the migration tooling (point readers to the
migration guide and reference the migration command name used in the repo), how
to enable the optional TUI (mention the --enable-tui flag or TUI config option),
and a short note about storage fallback policies (reference the
storage.fallback-policy/config key); include links to the detailed migration
guide and MCP schema docs already in the repo so users can follow up for full
instructions.
---
Duplicate comments:
In `@modules/cerebro/src/tui/redaction.rs`:
- Around line 19-20: TOKEN_RE currently matches any alphanumeric sequence >=24
chars which causes false positives; update the redaction logic by replacing or
refining TOKEN_RE in redaction.rs to be more specific (for example require known
token prefixes/suffixes, include context words like "token", "auth", "secret" in
lookbehind/lookahead, or enforce character classes/entropy constraints) and/or
add an additional context-aware check after a match (e.g., skip if it matches a
UUID-without-hyphens pattern or other whitelisted formats) so only true secrets
are redacted; ensure changes reference the TOKEN_RE symbol and the redaction
flow that consumes its matches.
In `@modules/cerebro/tests/tui_non_blocking_tests.rs`:
- Around line 51-53: The test currently drops the result of
shutdown_tx.send(true) and awaits handle.join() unbounded, which can hide send
failures and hang CI; change to assert the send succeeded (e.g., check
shutdown_tx.send(true) returns Ok or use expect/unwrap) and wrap the join await
with a timeout (use tokio::time::timeout with a short Duration and then
unwrap/expect both the timeout result and the join result) so failures and hangs
fail the test fast; update imports as needed and keep the existing
std::env::remove_var("CEREBRO_TUI_HEADLESS") cleanup.
In `@modules/cerebro/tests/tui_shutdown_tests.rs`:
- Around line 33-60: Both tests mutate process-wide env vars via
EnvVarGuard::set (in tui_exits_on_shutdown_signal and
tui_crash_isolated_from_caller) and can race when run concurrently; serialize
these mutations by adding a shared test lock and acquiring it before calling
EnvVarGuard::set. Concretely, introduce a static test mutex (e.g., TEST_ENV_LOCK
using once_cell::sync::Lazy<Mutex<()>> or similar) and lock it at the start of
each test before creating the EnvVarGuard, holding the lock until the guard is
dropped so env changes cannot leak between tests; update both tests to acquire
that mutex around their EnvVarGuard::set calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f2ed84aa-81fe-4d50-b5f5-91108f89a015
⛔ Files ignored due to path filters (2)
clients/agent-runtime/Cargo.lockis excluded by!**/*.lockmodules/cerebro/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
PRD-Corvus.mdPRD-English.mdREADME.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdlychee.tomlmodules/cerebro/Cargo.tomlmodules/cerebro/src/config.rsmodules/cerebro/src/main.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/src/server.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/src/tui/views/memory_explorer.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_event_emission_tests.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/tui_toggle_tests.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdopenspec/specs/cerebro/prompt_template.mdopenspec/specs/cerebro/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dependency-review
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/specs/cerebro/prompt_template.mdPRD-Corvus.mdPRD-English.mdREADME.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdmodules/cerebro/tests/tui_event_emission_tests.rsclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/src/tools.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/tests/storage_config_test.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/specs/cerebro/spec.mdmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/src/migration/mod.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/main.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdlychee.tomlmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/src/tui/mod.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/server.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/src/storage/mod.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/specs/cerebro/prompt_template.mdmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/storage/surreal.rsPRD-Corvus.mdmodules/cerebro/tests/tui_toggle_tests.rsPRD-English.mdREADME.mdmodules/cerebro/Cargo.tomlmodules/cerebro/src/config.rsmodules/cerebro/src/tui/views/memory_explorer.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
modules/cerebro/tests/tui_event_emission_tests.rsmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/main.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/server.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/src/config.rsmodules/cerebro/src/tui/views/memory_explorer.rs
🧠 Learnings (15)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Inspect existing module and adjacent tests before editing; define scope boundary with one concern per PR and avoid mixed feature+refactor+infra patches
Applied to files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdmodules/cerebro/tests/tui_shutdown_tests.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdmodules/cerebro/tests/tui_toggle_tests.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
modules/cerebro/tests/tui_event_emission_tests.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
modules/cerebro/tests/migration_report_test.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/cli_migration_test.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/server.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/src/main.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/server.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/main.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
modules/cerebro/src/tui/redaction.rsmodules/cerebro/src/server.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
modules/cerebro/src/main.rsmodules/cerebro/tests/cli_migration_test.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
modules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/src/tui/views/memory_explorer.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Applied to files:
modules/cerebro/src/server.rsmodules/cerebro/src/config.rs
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Maintain comprehensive agent metadata including name, description, purpose, and capabilities
Applied to files:
openspec/specs/cerebro/prompt_template.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Keep each iteration reversible with small commits and clear rollback strategy; validate assumptions with code search before implementing
Applied to files:
modules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
modules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
modules/cerebro/Cargo.toml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
modules/cerebro/Cargo.toml
🪛 LanguageTool
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.md
[style] ~19-~19: The double modal “requires embedded” is nonstandard (only accepted in certain dialects). Consider “to be embedded”.
Context: ...s. Rationale: The proposal requires embedded SurrealDB as the default for new deploy...
(NEEDS_FIXED)
PRD-English.md
[style] ~4-~4: Consider using a more concise synonym.
Context: ... written in Kotlin, designed to support a variety of applications including Android, iOS, an...
(A_VARIETY_OF)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.md
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 22-22: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 32-32: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 44-44: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 72-72: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 73-73: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 75-75: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 76-76: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 77-77: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 78-78: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 79-79: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 80-80: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 84-84: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 85-85: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 87-87: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 88-88: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 89-89: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 90-90: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 91-91: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 108-108: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 89-89: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
PRD-Corvus.md
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
PRD-English.md
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 12-12: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 27-27: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (39)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.md (1)
1-45: Task plan is clear and implementation-linked.The phased checklist is concrete and points to specific code/test paths, which makes verification and auditability straightforward.
openspec/specs/cerebro/prompt_template.md (1)
6-8: EN/ES parity handling is correctly documented.The explicit ES pending note plus tracking ID addresses the translation-gap requirement for user-facing guidance.
As per coding guidelines "**/*.{md,mdx}: For user-facing docs, check EN/ES parity or explicitly note pending translation gaps."modules/cerebro/src/tui/views/memory_explorer.rs (1)
22-32: Row rendering is allocation-friendly now.Using borrowed
&strfor cells removes the prior per-frame cloning overhead in this hot render path.modules/cerebro/src/tui/views/live_logs.rs (1)
21-37: Live log item formatting looks good and efficient.The status fallback and borrowed string usage keep this render path lightweight per refresh tick.
modules/cerebro/src/tools.rs (2)
10-63: LGTM on ToolRedaction structure.The static slice approach for allowed fields avoids heap allocations and the catch-all at lines 57-60 returns empty allowlists for unknown tools—secure-by-default behavior.
295-359: LGTM—mem_timelinebranch now present.The extractor aligns with the allowlist at line 45. The fallback to counting
itemsarray (lines 338-342) handles both explicititems_countand derived counts gracefully.modules/cerebro/tests/migration_legacy_test.rs (1)
14-24: LGTM on normalization assertions.Tests correctly verify that
memory:01→01(prefix stripped) whilesession:01is preserved as-is, matching thenormalize_memory_idimplementation inlegacy.rs.openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.md (1)
1-83: LGTM—Proposal aligns with implementation.Scope, risks, and success criteria accurately reflect the migration tooling and embedded SurrealDB default now implemented in
modules/cerebro/src/migration/*andconfig.rs. The rollback plan and mitigation strategies are practical.openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.md (1)
1-168: LGTM—Design doc is technically accurate and well-structured.Architecture decisions, sequence diagrams, and contract notes align with the implementation. The drill-in retrieval pattern (compact search → selective full fetch) matches the tool semantics in
tools.rs.modules/cerebro/tests/migration_report_test.rs (1)
1-87: LGTM—Previous review concerns addressed.Fixture paths now use
env!("CARGO_MANIFEST_DIR")(line 84), and both tests assert fullto_json_value()equality against fixtures, locking down the complete serialization contract.modules/cerebro/tests/tui_event_emission_tests.rs (1)
18-27: Good event-stream test hardening.Timeout-bounded receives and explicit
ToolCallEventKindchecks make these async tests fail fast and deterministically.Also applies to: 46-55
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.md (1)
110-127: Spec scope alignment looks correct now.The optional TUI requirement is explicitly compatible with migration tooling and MCP availability, which keeps this spec delta coherent with the PR scope.
modules/cerebro/tests/migration_workflow_test.rs (1)
20-31: Strong workflow coverage with typed status checks.Both success and mismatch paths are validated with
MigrationStatusenums, which improves test robustness.Also applies to: 45-62
clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md (1)
94-173: Docs update is well-structured and parity-aware.The new operational sections include explicit ES-pending markers, which keeps translation tracking clear.
modules/cerebro/tests/embedded_storage_test.rs (1)
39-50: Nice test hardening on delete and export behavior.Post-delete visibility checks and export assertions improve regression coverage for embedded storage semantics.
Also applies to: 73-78
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.md (1)
1-167: LGTM!Design document is well-structured with clear architecture decisions, rationale, data flow diagrams, and failure mode documentation. The deny-by-default redaction policy and TUI isolation approach align with security-first principles.
modules/cerebro/src/main.rs (2)
10-65: LGTM!The startup flow correctly:
- Loads config from file/env before service initialization
- Spawns TUI in a background task to avoid blocking MCP accept loop
- Validates no network listeners before TUI launch
- Handles TUI failures gracefully without crashing the server
67-105: LGTM!Shutdown signal handling is correctly implemented with watch channel for coordination between signal handler and graceful shutdown. The
wait_for_shutdownhelper properly awaits state changes.modules/cerebro/src/migration/legacy.rs (1)
54-67: LGTM!Error handling provides good context with path display and appropriate error type mapping (Storage for IO, Validation for JSON parse).
modules/cerebro/src/migration/mod.rs (1)
26-46: LGTM!
import_legacy_exportcorrectly handles dry-run mode by skipping writes, and produces consistent reports for both dry-run and actual import paths.modules/cerebro/src/tui/mod.rs (2)
362-375: LGTM!Storage refresh now performs a single query and clones the result for both
memory_itemsandtimeline_items, addressing the previous duplicate query concern.
184-196: LGTM!Exponential backoff on storage query failures with a 5-second cap is a good resilience pattern that prevents hammering a failing storage backend.
modules/cerebro/Cargo.toml (2)
42-44: LGTM!TUI feature flag correctly gates the ratatui and crossterm dependencies as optional, avoiding binary size impact when TUI is not needed.
33-33: surrealdb version 3.0.4 is confirmed to exist on crates.io. The dependency specification with thekv-rocksdbfeature is valid and resolvable.modules/cerebro/src/tui/redaction.rs (2)
75-85: LGTM!
redact_textproperly combines word-boundary pattern matching with secret pattern detection, addressing the previous review concern about substring matching.
29-52: LGTM!
from_configcorrectly normalizes field names to lowercase and enforces a minimum payload size. The word-boundary patterns useregex::escapeto prevent regex injection.openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.md (1)
141-167: Interface contract looks good and matches implementation.The
SurrealConfigstruct definition correctly showspassword: Option<SecretString>, which aligns with the actual implementation inmodules/cerebro/src/config.rs. The serialization safeguards (skip_serializing viasecret_string_opt) are implemented in code but not explicitly noted here—consider adding a brief note that credentials are never serialized back to prevent accidental leakage in logs or exports.modules/cerebro/src/server.rs (3)
183-198: Error redaction now applied correctly.The error path at line 184 now uses
self.redaction.redact_text(&error.to_string())before publishing to the event bus, addressing the previous concern about leaking sensitive payloads in tool failure messages.
60-71: Service initialization wires TUI components correctly.EventBus and RedactionPolicy are constructed from
config.tui, and the tools are initialized with the storage clone. The field ordering in the struct initialization is clean. No secrets exposed during construction.
129-150: Redaction ordering is correct: args redacted before tool execution.The flow correctly extracts safe args via
extract_safe_args(line 131-134) and applies allowlist redaction (line 135-137) before the tool executes. This ensures theStartedevent never contains sensitive input data.openspec/specs/cerebro/spec.md (1)
16-20: Constraints correctly scope embedded SurrealDB and TUI.The constraints clearly establish that embedded SurrealDB is a Cerebro deployment mode only (not runtime-local), TUI is optional and non-blocking, and migration tooling is required. This aligns with the implementation.
modules/cerebro/src/storage/mod.rs (2)
355-398: Fallback logic is explicit and well-structured.The fallback handling now:
- Validates storage config upfront (line 358)
- Logs warnings with structured fields when fallback activates (lines 364-367, 371-374, 384-387)
- Wraps both primary and fallback errors with context (lines 377-381, 390-394)
This addresses the previous review about inconsistent error context and aligns with the learning to prefer explicit errors over silent fallback.
17-18: No action required—code is compatible with surrealdb 3.x.The
#[derive(SurrealValue)]macro with#[surreal(crate = "surrealdb::types")]is properly configured for surrealdb 3.0.4. The dependency versions are correctly resolved in Cargo.lock, and a recent commit specifically upgraded the module to 3.x without introducing compilation issues.modules/cerebro/src/storage/surreal.rs (2)
53-57:RemoteSurrealintentionally returnsNotImplemented.This is gated at config validation time (
validate_storagerejectsStorageMode::RemoteSurreal), so this constructor should never be called in practice. The approach is acceptable—fail early at config validation rather than here. The past review concern is addressed by the config-level guard.
65-132: Transaction handling now uses explicit BEGIN/COMMIT.The batch import correctly wraps all UPSERT statements in a single transaction (lines 67, 121), addressing the previous review concern about partial commits corrupting data. All collections are committed atomically or not at all.
modules/cerebro/src/config.rs (4)
383-389: Secret serialization now correctly prevented.The
serializefunction always returnsNone, ensuringSecretStringvalues (auth_token, audit_token, password) are never serialized back out. This addresses the previous review concern about credential leakage.
292-328: Embedded SurrealDB validation enforces security constraints.The validation correctly:
- Requires loopback binding unless explicitly overridden (lines 302-306)
- Requires both username AND password (lines 308-325)
This matches the spec requirements for embedded SurrealDB authentication and loopback binding.
272-290: RemoteSurreal rejected early at config validation.Both
StorageMode::RemoteSurreal(lines 274-278) andStorageFallback::RemoteSurreal(lines 283-287) are rejected withNotImplementederrors during validation. This is the correct place to fail—before any storage initialization attempt.
233-250: Environment override handling is safe.Tokens are trimmed and checked for empty before assignment (lines 235-238, 241-244). The
env_flaghelper (lines 372-377) correctly interprets boolean-like values. No secrets are logged during override application.
| async fn search( | ||
| &self, | ||
| query: &str, | ||
| limit: usize, | ||
| include_deleted: bool, | ||
| scope: Option<&str>, | ||
| topic_key: Option<&str>, | ||
| ) -> Result<Vec<MemoryRecord>, CerebroError> { | ||
| let mut records: Vec<MemoryRecord> = self | ||
| .db | ||
| .select::<Vec<MemoryRecord>>("memory") | ||
| .await | ||
| .map_err(|err| CerebroError::Storage(format!("surrealdb search failed: {err}")))?; | ||
|
|
||
| let query = query.to_ascii_lowercase(); | ||
| records.retain(|record| { | ||
| if !include_deleted && record.deleted { | ||
| return false; | ||
| } | ||
| if let Some(scope) = scope { | ||
| if record.scope != scope { | ||
| return false; | ||
| } | ||
| } | ||
| if let Some(topic_key) = topic_key { | ||
| if record.topic_key != topic_key { | ||
| return false; | ||
| } | ||
| } | ||
| let haystack = format!("{} {}", record.summary, record.topic_key).to_ascii_lowercase(); | ||
| haystack.contains(&query) | ||
| }); | ||
|
|
||
| records.sort_by(|a, b| b.timestamp.cmp(&a.timestamp)); | ||
| records.truncate(limit); | ||
| Ok(records) | ||
| } | ||
|
|
||
| async fn count(&self) -> Result<usize, CerebroError> { | ||
| let records: Vec<MemoryRecord> = self | ||
| .db | ||
| .select::<Vec<MemoryRecord>>("memory") | ||
| .await | ||
| .map_err(|err| CerebroError::Storage(format!("surrealdb count failed: {err}")))?; | ||
| Ok(records.len()) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Search and count load all records into memory—consider server-side filtering.
Both search (line 349-353) and count (line 380-384) fetch the entire memory table and filter/count client-side. For large datasets, this will degrade performance and memory usage.
Consider using SurrealQL WHERE clauses and count() aggregate:
-- For search with filters
SELECT * FROM memory WHERE !deleted AND scope = $scope AND (summary CONTAINS $query OR topic_key CONTAINS $query) ORDER BY timestamp DESC LIMIT $limit
-- For count
SELECT count() FROM memory GROUP ALLThis is acceptable for phase 1 with small datasets but should be addressed before production scale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/storage/surreal.rs` around lines 341 - 386, The search
and count implementations currently load all records via
self.db.select("memory") and filter/count in Rust (functions search and count);
change them to run SurrealQL server-side queries using parameterized WHERE
clauses and COUNT() to avoid pulling the whole table: for search build a SELECT
... FROM memory WHERE ... (respect include_deleted, scope, topic_key, and a
case-insensitive CONTAINS on summary or topic_key), ORDER BY timestamp DESC
LIMIT $limit and execute it via self.db.query or the appropriate client call,
mapping results to Vec<MemoryRecord> and preserving CerebroError mapping; for
count run a SELECT count() FROM memory with the same WHERE predicates (or GROUP
ALL) and return the numeric result instead of records.len().
| pub fn extract_safe_args(&self, tool: &str, payload: &Value) -> Option<Value> { | ||
| match tool { | ||
| "mem_save" => { | ||
| let input: ToolInput<MemSaveRequest> = serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "scope": input.input.scope, | ||
| "topic_key": input.input.topic_key, | ||
| })) | ||
| } | ||
| "mem_search" => { | ||
| let input: ToolInput<MemSearchRequest> = serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "limit": input.input.limit, | ||
| "scope": input.input.scope, | ||
| "topic_key": input.input.topic_key, | ||
| "include_deleted": input.input.include_deleted, | ||
| })) | ||
| } | ||
| "mem_delete" => { | ||
| let input: ToolInput<MemDeleteRequest> = serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "memory_id": input.input.memory_id, | ||
| "topic_key": input.input.topic_key, | ||
| "hard_delete": input.input.hard_delete, | ||
| })) | ||
| } | ||
| "mem_get_observation" => { | ||
| let input: ToolInput<MemGetObservationRequest> = | ||
| serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "memory_id": input.input.memory_id, | ||
| "include_deleted": input.input.include_deleted, | ||
| })) | ||
| } | ||
| "mem_update" => { | ||
| let input: ToolInput<MemUpdateRequest> = serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "memory_id": input.input.memory_id, | ||
| })) | ||
| } | ||
| "mem_suggest_topic_key" => { | ||
| let input: ToolInput<MemSuggestTopicKeyRequest> = | ||
| serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "seed": input.input.seed, | ||
| "scope": input.input.scope, | ||
| })) | ||
| } | ||
| "mem_timeline" => { | ||
| let input: ToolInput<MemTimelineRequest> = serde_json::from_value(payload.clone()).ok()?; | ||
| Some(json!({ | ||
| "memory_id": input.input.memory_id, | ||
| "before": input.input.before, | ||
| "after": input.input.after, | ||
| "include_deleted": input.input.include_deleted, | ||
| })) | ||
| } | ||
| "mem_stats" => Some(json!({})), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unnecessary payload.clone() on each branch.
Every match arm clones the entire payload before parsing. Since serde_json::from_value consumes the value anyway, you can pass payload directly (taking ownership) to avoid the allocation.
♻️ Example fix for one branch
- pub fn extract_safe_args(&self, tool: &str, payload: &Value) -> Option<Value> {
+ pub fn extract_safe_args(&self, tool: &str, payload: Value) -> Option<Value> {
match tool {
"mem_save" => {
- let input: ToolInput<MemSaveRequest> = serde_json::from_value(payload.clone()).ok()?;
+ let input: ToolInput<MemSaveRequest> = serde_json::from_value(payload).ok()?;If callers need to retain the original payload, consider Cow or accept the clone cost explicitly at call sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/tools.rs` around lines 233 - 293, The function
extract_safe_args is cloning payload in every match arm (payload.clone()) which
is unnecessary; change the signature to take payload by value (fn
extract_safe_args(&self, tool: &str, payload: Value) -> Option<Value>) and
remove all payload.clone() calls, using serde_json::from_value(payload) in each
branch instead, and update all call sites to pass ownership (or if call-site
changes are infeasible, clone once at the start: let payload = payload.clone();
and use that single owned Value for all serde_json::from_value calls). Ensure
references to extract_safe_args, payload.clone(), and serde_json::from_value are
updated accordingly.
| ### Verdict | ||
| PASS |
There was a problem hiding this comment.
Add blank line after "Verdict" heading.
MD022 requires blank lines around headings.
🔧 Fix
### Verdict
+
PASS📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Verdict | |
| PASS | |
| ### Verdict | |
| PASS |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.md`
around lines 83 - 84, The "### Verdict" heading in verify-report.md is missing a
blank line after it; insert a single empty line between the "### Verdict"
heading and the following "PASS" text so the heading is separated from its
content (i.e., change "### Verdict\nPASS" to "### Verdict\n\nPASS") to satisfy
MD022.
| ## 1. Introduction and Objectives | ||
| The Corvus project is a Gradle-based multi-module system written in Kotlin, tailored for multi-platform development. It supports Android, iOS, and Compose desktop applications, with a strong emphasis on security, performance, and modularity. The project integrates the Cerebro memory system, a Rust-based solution designed for AI agents. | ||
|
|
||
| ### Objectives | ||
| - Deliver a modular and scalable architecture for multi-platform development. | ||
| - Prioritize security in every aspect of the system. | ||
| - Optimize performance while maintaining code quality. | ||
| - Enable seamless integration with AI agents and memory systems. | ||
| - Support Test-Driven Development (TDD) and maintain high code quality. | ||
|
|
||
| ## 2. Key Features | ||
| - **Multi-Platform Support**: Dedicated modules for Android, iOS, and Compose desktop applications. | ||
| - **Cerebro Memory System**: A high-performance Rust-based memory module with SurrealDB for multi-model storage. | ||
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs for consistency. | ||
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | ||
| - **Performance Optimization**: Efficient algorithms, lazy initialization, and profiling. | ||
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | ||
|
|
||
| ## 3. Architecture Overview | ||
| The Corvus project is structured as follows: | ||
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | ||
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | ||
| - **Gradle**: Custom build logic and version catalogs. | ||
| - **Docs**: Comprehensive documentation for developers. | ||
|
|
||
| ## 4. Security and Performance Principles | ||
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | ||
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | ||
|
|
||
| ## 5. Development Workflow | ||
| - Follow TDD: Red -> Green -> Refactor. | ||
| - Use Makefile commands for streamlined development. | ||
| - Maintain code quality with Spotless, Detekt, and other tools. | ||
|
|
||
| ## 6. Integration and Testing |
There was a problem hiding this comment.
Fix markdown heading spacing to satisfy lint (MD022).
Several headings are missing the required blank line separation below them. This will keep docs lint-clean and reduce CI/doc-noise.
Suggested patch pattern
## 1. Introduction and Objectives
+
The Corvus project is a Gradle-based multi-module system written in Kotlin, tailored for multi-platform development.
@@
### Objectives
+
- Deliver a modular and scalable architecture for multi-platform development.
@@
## 2. Key Features
+
- **Multi-Platform Support**: Dedicated modules for Android, iOS, and Compose desktop applications.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 1. Introduction and Objectives | |
| The Corvus project is a Gradle-based multi-module system written in Kotlin, tailored for multi-platform development. It supports Android, iOS, and Compose desktop applications, with a strong emphasis on security, performance, and modularity. The project integrates the Cerebro memory system, a Rust-based solution designed for AI agents. | |
| ### Objectives | |
| - Deliver a modular and scalable architecture for multi-platform development. | |
| - Prioritize security in every aspect of the system. | |
| - Optimize performance while maintaining code quality. | |
| - Enable seamless integration with AI agents and memory systems. | |
| - Support Test-Driven Development (TDD) and maintain high code quality. | |
| ## 2. Key Features | |
| - **Multi-Platform Support**: Dedicated modules for Android, iOS, and Compose desktop applications. | |
| - **Cerebro Memory System**: A high-performance Rust-based memory module with SurrealDB for multi-model storage. | |
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs for consistency. | |
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | |
| - **Performance Optimization**: Efficient algorithms, lazy initialization, and profiling. | |
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | |
| ## 3. Architecture Overview | |
| The Corvus project is structured as follows: | |
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | |
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | |
| - **Gradle**: Custom build logic and version catalogs. | |
| - **Docs**: Comprehensive documentation for developers. | |
| ## 4. Security and Performance Principles | |
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | |
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | |
| ## 5. Development Workflow | |
| - Follow TDD: Red -> Green -> Refactor. | |
| - Use Makefile commands for streamlined development. | |
| - Maintain code quality with Spotless, Detekt, and other tools. | |
| ## 6. Integration and Testing | |
| ## 1. Introduction and Objectives | |
| The Corvus project is a Gradle-based multi-module system written in Kotlin, tailored for multi-platform development. It supports Android, iOS, and Compose desktop applications, with a strong emphasis on security, performance, and modularity. The project integrates the Cerebro memory system, a Rust-based solution designed for AI agents. | |
| ### Objectives | |
| - Deliver a modular and scalable architecture for multi-platform development. | |
| - Prioritize security in every aspect of the system. | |
| - Optimize performance while maintaining code quality. | |
| - Enable seamless integration with AI agents and memory systems. | |
| - Support Test-Driven Development (TDD) and maintain high code quality. | |
| ## 2. Key Features | |
| - **Multi-Platform Support**: Dedicated modules for Android, iOS, and Compose desktop applications. | |
| - **Cerebro Memory System**: A high-performance Rust-based memory module with SurrealDB for multi-model storage. | |
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs for consistency. | |
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | |
| - **Performance Optimization**: Efficient algorithms, lazy initialization, and profiling. | |
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | |
| ## 3. Architecture Overview | |
| The Corvus project is structured as follows: | |
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | |
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | |
| - **Gradle**: Custom build logic and version catalogs. | |
| - **Docs**: Comprehensive documentation for developers. | |
| ## 4. Security and Performance Principles | |
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | |
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | |
| ## 5. Development Workflow | |
| - Follow TDD: Red -> Green -> Refactor. | |
| - Use Makefile commands for streamlined development. | |
| - Maintain code quality with Spotless, Detekt, and other tools. | |
| ## 6. Integration and Testing | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD-Corvus.md` around lines 3 - 37, Several Markdown headings in
PRD-Corvus.md (e.g., "## 1. Introduction and Objectives", "### Objectives", "##
2. Key Features", "## 3. Architecture Overview", "## 4. Security and Performance
Principles", "## 5. Development Workflow", and "## 6. Integration and Testing")
lack the required blank line after the heading; update the file so each heading
line is followed by a single blank line before the next paragraph or list to
satisfy MD022 linting. Ensure you add the blank line immediately after each
heading string listed above (and any other headings in the document) without
changing heading text or content.
| ## 1. Introduction and Objectives | ||
| The Corvus project is a Gradle-based multi-module system written in Kotlin, designed to support a variety of applications including Android, iOS, and Compose desktop apps. It emphasizes centralized build configurations, custom plugins, and version catalogs to ensure consistency and maintainability. The project also integrates a high-performance Rust-based memory system (Cerebro) for AI agents. | ||
|
|
||
| ### Objectives | ||
| - Provide a modular and scalable architecture for multi-platform development. | ||
| - Ensure security and performance as primary principles. | ||
| - Facilitate seamless integration with AI agents and memory systems. | ||
| - Support Test-Driven Development (TDD) and maintain high code quality. | ||
|
|
||
| ## 2. Key Features | ||
| - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. | ||
| - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. | ||
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. | ||
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | ||
| - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. | ||
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | ||
|
|
||
| ## 3. Architecture Overview | ||
| The project is structured into the following components: | ||
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | ||
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | ||
| - **Gradle**: Custom build logic and version catalogs. | ||
| - **Docs**: Comprehensive documentation for developers. | ||
|
|
||
| ## 4. Security and Performance Principles | ||
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | ||
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | ||
|
|
||
| ## 5. Development Workflow | ||
| - Follow TDD: Red -> Green -> Refactor. | ||
| - Use Makefile commands for streamlined development. | ||
| - Maintain code quality with Spotless, Detekt, and other tools. | ||
|
|
||
| ## 6. Integration and Testing | ||
| - Integrate with AI agents via the MCP JSON-RPC protocol. | ||
| - Test modules independently and as part of the whole system. | ||
| - Ensure high test coverage and adherence to coding standards. |
There was a problem hiding this comment.
Add blank lines after all headings (MD022).
Static analysis flags lines 3, 6, 12, 20, 27, 31, and 36 for missing blank lines below headings.
🔧 Example fix pattern
## 1. Introduction and Objectives
+
The Corvus project is a Gradle-based...Apply to all flagged headings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 1. Introduction and Objectives | |
| The Corvus project is a Gradle-based multi-module system written in Kotlin, designed to support a variety of applications including Android, iOS, and Compose desktop apps. It emphasizes centralized build configurations, custom plugins, and version catalogs to ensure consistency and maintainability. The project also integrates a high-performance Rust-based memory system (Cerebro) for AI agents. | |
| ### Objectives | |
| - Provide a modular and scalable architecture for multi-platform development. | |
| - Ensure security and performance as primary principles. | |
| - Facilitate seamless integration with AI agents and memory systems. | |
| - Support Test-Driven Development (TDD) and maintain high code quality. | |
| ## 2. Key Features | |
| - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. | |
| - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. | |
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. | |
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | |
| - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. | |
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | |
| ## 3. Architecture Overview | |
| The project is structured into the following components: | |
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | |
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | |
| - **Gradle**: Custom build logic and version catalogs. | |
| - **Docs**: Comprehensive documentation for developers. | |
| ## 4. Security and Performance Principles | |
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | |
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | |
| ## 5. Development Workflow | |
| - Follow TDD: Red -> Green -> Refactor. | |
| - Use Makefile commands for streamlined development. | |
| - Maintain code quality with Spotless, Detekt, and other tools. | |
| ## 6. Integration and Testing | |
| - Integrate with AI agents via the MCP JSON-RPC protocol. | |
| - Test modules independently and as part of the whole system. | |
| - Ensure high test coverage and adherence to coding standards. | |
| ## 1. Introduction and Objectives | |
| The Corvus project is a Gradle-based multi-module system written in Kotlin, designed to support a variety of applications including Android, iOS, and Compose desktop apps. It emphasizes centralized build configurations, custom plugins, and version catalogs to ensure consistency and maintainability. The project also integrates a high-performance Rust-based memory system (Cerebro) for AI agents. | |
| ### Objectives | |
| - Provide a modular and scalable architecture for multi-platform development. | |
| - Ensure security and performance as primary principles. | |
| - Facilitate seamless integration with AI agents and memory systems. | |
| - Support Test-Driven Development (TDD) and maintain high code quality. | |
| ## 2. Key Features | |
| - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. | |
| - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. | |
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. | |
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | |
| - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. | |
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | |
| ## 3. Architecture Overview | |
| The project is structured into the following components: | |
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | |
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | |
| - **Gradle**: Custom build logic and version catalogs. | |
| - **Docs**: Comprehensive documentation for developers. | |
| ## 4. Security and Performance Principles | |
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | |
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | |
| ## 5. Development Workflow | |
| - Follow TDD: Red -> Green -> Refactor. | |
| - Use Makefile commands for streamlined development. | |
| - Maintain code quality with Spotless, Detekt, and other tools. | |
| ## 6. Integration and Testing | |
| - Integrate with AI agents via the MCP JSON-RPC protocol. | |
| - Test modules independently and as part of the whole system. | |
| - Ensure high test coverage and adherence to coding standards. |
🧰 Tools
🪛 LanguageTool
[style] ~4-~4: Consider using a more concise synonym.
Context: ... written in Kotlin, designed to support a variety of applications including Android, iOS, an...
(A_VARIETY_OF)
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 12-12: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 27-27: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD-English.md` around lines 3 - 39, The markdown headings (e.g., "## 1.
Introduction and Objectives", "### Objectives", "## 2. Key Features", "## 3.
Architecture Overview", "## 4. Security and Performance Principles", "## 5.
Development Workflow", "## 6. Integration and Testing") are missing a blank line
below them which triggers MD022; add a single blank line immediately after each
of these headings so there is an empty line between the heading line and the
following paragraph or list item throughout the document.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (15)
PRD-English.md (1)
3-37:⚠️ Potential issue | 🟡 MinorFix MD022: headings need a blank line below
This lint issue was already reported on prior revisions and is still present.
Suggested patch
## 1. Introduction and Objectives + The Corvus project is a Gradle-based multi-module system written in Kotlin, designed to support a variety of applications including Android, iOS, and Compose desktop apps. It emphasizes centralized build configurations, custom plugins, and version catalogs to ensure consistency and maintainability. The project also integrates a high-performance Rust-based memory system (Cerebro) for AI agents. ### Objectives + - Provide a modular and scalable architecture for multi-platform development. - Ensure security and performance as primary principles. - Facilitate seamless integration with AI agents and memory systems. - Support Test-Driven Development (TDD) and maintain high code quality. ## 2. Key Features + - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. ## 3. Architecture Overview + The project is structured into the following components: - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. - **Gradle**: Custom build logic and version catalogs. - **Docs**: Comprehensive documentation for developers. ## 4. Security and Performance Principles + - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. ## 5. Development Workflow + - Follow TDD: Red -> Green -> Refactor. - Use Makefile commands for streamlined development. - Maintain code quality with Spotless, Detekt, and other tools. ## 6. Integration and Testing + - Integrate with AI agents via the MCP JSON-RPC protocol. - Test modules independently and as part of the whole system. - Ensure high test coverage and adherence to coding standards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PRD-English.md` around lines 3 - 37, The file has multiple headings that violate MD022 (no blank line after a heading); for each heading like "## 1. Introduction and Objectives", "### Objectives", "## 2. Key Features", "## 3. Architecture Overview", "## 4. Security and Performance Principles", "## 5. Development Workflow", and "## 6. Integration and Testing", insert a single blank line immediately after the heading line so every heading is followed by an empty line before the next content paragraph or list; update each heading occurrence accordingly to resolve the MD022 lint error.modules/cerebro/tests/cli_migration_test.rs (1)
63-65:⚠️ Potential issue | 🟡 MinorMake the mismatch mutation mandatory in the test precondition.
At Lines 63-65, the optional branch can skip mutation silently if
memoryis missing/empty, weakening the test guarantee.Suggested fix
- if let Some(array) = json.get_mut("memory").and_then(Value::as_array_mut) { - array.pop(); - } + if let Some(array) = json.get_mut("memory").and_then(Value::as_array_mut) { + assert!( + array.pop().is_some(), + "fixture `memory` array is unexpectedly empty" + ); + } else { + panic!("fixture missing `memory` array"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/cli_migration_test.rs` around lines 63 - 65, The test currently optionally mutates the JSON with json.get_mut("memory").and_then(Value::as_array_mut) { array.pop(); } which can silently skip the mutation; make the mismatch mutation mandatory by asserting that the "memory" field exists and is a non-empty array, then perform the mutation (e.g., obtain a mutable array via json.get_mut("memory").and_then(Value::as_array_mut).expect(...) and call pop()), so the test fails fast if the precondition isn't met and the mutation always occurs.modules/cerebro/src/tools.rs (3)
233-293: 🧹 Nitpick | 🔵 TrivialPer-branch
payload.clone()allocates unnecessarily.Each match arm clones the entire payload before
serde_json::from_valueconsumes it. Consider takingpayload: Valueby value to avoid the allocation, or accept the cost explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tools.rs` around lines 233 - 293, The function extract_safe_args currently takes payload: &Value and calls payload.clone() in every match arm before serde_json::from_value, causing unnecessary allocations; change the signature to extract_safe_args(&self, tool: &str, payload: Value) and remove the .clone() calls so serde_json::from_value(payload) consumes the value directly, or alternately only clone in arms that need it; update all references to payload.clone() in extract_safe_args (and any callers) to pass ownership of the Value instead of a reference.
39-42:⚠️ Potential issue | 🟠 MajorPrivacy risk:
seedfield contains user-provided text.The
seedfield formem_suggest_topic_keycontains user input processed to generate topic slugs. Including it inallowed_arg_fieldsexposes user content in TUI logs. Onlyscopeshould remain in the allowlist.Suggested fix
"mem_suggest_topic_key" => Self { - allowed_arg_fields: &["seed", "scope"], + allowed_arg_fields: &["scope"], allowed_output_fields: &["topic_key", "candidates_count"], },As per coding guidelines
**/*: "Validate input boundaries, auth/authz implications, and secret management."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tools.rs` around lines 39 - 42, The mem_suggest_topic_key allowlist currently exposes user text via the "seed" arg; update the mapping for the "mem_suggest_topic_key" case in tools.rs so allowed_arg_fields only contains "scope" (remove "seed"), leaving allowed_output_fields unchanged; locate the match/constructor where mem_suggest_topic_key is created and modify the allowed_arg_fields slice to ["scope"] to eliminate logging of user-provided seed text.
273-280:⚠️ Potential issue | 🟠 MajorAlign
extract_safe_argswith redaction policy: removeseed.This branch extracts
seed(user-provided text) into the safe args output, which contradicts privacy-safe logging. Removeseedto match the correctedallowed_arg_fields.Suggested fix
"mem_suggest_topic_key" => { let input: ToolInput<MemSuggestTopicKeyRequest> = serde_json::from_value(payload.clone()).ok()?; Some(json!({ - "seed": input.input.seed, "scope": input.input.scope, })) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tools.rs` around lines 273 - 280, In the "mem_suggest_topic_key" branch of extract_safe_args, stop exposing the user-provided seed: parse the payload into ToolInput<MemSuggestTopicKeyRequest> as before (using payload.clone()) but build the returned safe-args JSON without the "seed" field (only include "scope" or any non-sensitive fields), so remove "seed" from the json! output to align with allowed_arg_fields and the redaction policy.modules/cerebro/src/tui/views/mod.rs (1)
32-35:⚠️ Potential issue | 🟡 MinorTabs area height insufficient for bordered block with padding.
Constraint::Length(2)cannot accommodate the Tabs widget withBorders::ALL(1 row top border + 1 row top padding + 1 row content + 1 row bottom padding + 1 row bottom border = 5 rows minimum). The suggestedLength(3)is also insufficient. Increase to at leastLength(5).Suggested fix
let chunks = Layout::default() .direction(Direction::Vertical) - .constraints([Constraint::Length(2), Constraint::Length(1)]) + .constraints([Constraint::Length(5), Constraint::Length(1)]) .split(area);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tui/views/mod.rs` around lines 32 - 35, The vertical layout's first constraint (Constraint::Length(2)) is too small for the Tabs widget which uses Borders::ALL and padding; update the constraints passed to Layout::default().constraints(...) (the array containing Constraint::Length(2), Constraint::Length(1)) to use at least Constraint::Length(5) for the tabs area so the Tabs widget (with Borders::ALL and internal padding) has enough rows to render correctly.modules/cerebro/tests/migration_legacy_test.rs (1)
4-12:⚠️ Potential issue | 🟡 MinorFixture paths are cwd-dependent; use
CARGO_MANIFEST_DIR.Plain relative paths can fail outside standard
cargo testcontext. Other tests in this PR (e.g.,migration_report_test.rs) useenv!("CARGO_MANIFEST_DIR")for deterministic fixture loading.Suggested fix
+fn fixture_path(relative: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(relative) +} + #[test] fn parses_legacy_export_fixture() { - let path = PathBuf::from("tests/fixtures/legacy/legacy_export.json"); + let path = fixture_path("tests/fixtures/legacy/legacy_export.json"); let export = read_legacy_export(&path).expect("legacy export should parse");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/migration_legacy_test.rs` around lines 4 - 12, The test parses_legacy_export_fixture uses a relative PathBuf which is cwd-dependent; change the PathBuf construction to use the crate root via env!("CARGO_MANIFEST_DIR") and join the fixture path so the test loads deterministically. Locate the test function parses_legacy_export_fixture and replace the PathBuf::from("tests/fixtures/legacy/legacy_export.json") call with a PathBuf built from env!("CARGO_MANIFEST_DIR") joined with "tests/fixtures/legacy/legacy_export.json" so read_legacy_export(&path) always finds the fixture regardless of working directory.modules/cerebro/tests/tui_toggle_tests.rs (1)
72-72:⚠️ Potential issue | 🟠 MajorBound
handle.join()with a timeout to prevent hangs.Line 72 waits unboundedly; if shutdown regresses, this test can hang the suite instead of failing fast.
Suggested patch
use tokio::sync::watch; +use tokio::time::{timeout, Duration}; @@ - handle.join().await.expect("tui join should succeed"); + timeout(Duration::from_secs(1), handle.join()) + .await + .expect("tui join timeout") + .expect("tui join should succeed");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/tui_toggle_tests.rs` at line 72, The test currently awaits handle.join().await without a timeout, which can hang; wrap the join in a bounded timeout (e.g., using tokio::time::timeout with a suitable Duration) and assert that the timeout did not elapse before unwrapping the join result; update the await call on handle.join() in tui_toggle_tests.rs to use tokio::time::timeout(..., handle.join()).await and fail the test if the timeout occurs while still preserving the existing expect("tui join should succeed") on the join result.openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md (1)
8-90:⚠️ Potential issue | 🟡 MinorNormalize section heading levels and Verdict spacing.
Line 8 still jumps from H1 to H3, and Line 89 has no blank line below the heading before content on Line 90. This keeps MD001/MD022 warnings active.
Suggested patch
-### Completeness +## Completeness @@ -### Build & Tests Execution +## Build & Tests Execution @@ -### Spec Compliance Matrix +## Spec Compliance Matrix @@ -### Correctness (Static — Structural Evidence) +## Correctness (Static — Structural Evidence) @@ -### Coherence (Design) +## Coherence (Design) @@ -### Issues Found +## Issues Found @@ -### Verdict +## Verdict + PASS WITH WARNINGSAs per coding guidelines,
**/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md` around lines 8 - 90, The document jumps heading levels (H1 to H3) and lacks a blank line after the "Verdict" heading causing MD001/MD022 warnings; update the headings under the top-level title so section headings like "Completeness", "Build & Tests Execution", "Spec Compliance Matrix", "Correctness (Static — Structural Evidence)", "Coherence (Design)", "Issues Found", and "Verdict" use consistent H2 (i.e., "##") or H3 levels as appropriate (e.g., change "### Completeness" to "## Completeness" to avoid skipping levels) and insert a single blank line after the "Verdict" heading before the verdict text to ensure proper spacing. Ensure changes touch the heading lines with the exact headings mentioned above so linters MD001/MD022 pass.modules/cerebro/tests/tui_shutdown_tests.rs (1)
34-60:⚠️ Potential issue | 🟠 MajorSerialize env-var mutations in these async tests.
Line 34 and Line 58 mutate process-global env without a shared lock. Parallel execution can cause cross-test interference (especially with
CEREBRO_TUI_TEST_CRASH) even with RAII restoration.Suggested patch
+use std::sync::Mutex; @@ +static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ async fn tui_exits_on_shutdown_signal() { + let _guard = ENV_LOCK.lock().expect("env lock"); let _headless = EnvVarGuard::set("CEREBRO_TUI_HEADLESS", "1"); @@ async fn tui_crash_isolated_from_caller() { + let _guard = ENV_LOCK.lock().expect("env lock"); let _headless = EnvVarGuard::set("CEREBRO_TUI_HEADLESS", "1"); let _crash = EnvVarGuard::set("CEREBRO_TUI_TEST_CRASH", "1");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/tui_shutdown_tests.rs` around lines 34 - 60, The tests mutate process-global env vars via EnvVarGuard::set (e.g., in the test functions that set "CEREBRO_TUI_HEADLESS" and "CEREBRO_TUI_TEST_CRASH") and must serialize those mutations to avoid cross-test interference; fix by introducing a shared global async lock (e.g., a static Lazy<tokio::sync::Mutex<()>> or similar) and acquire the lock at the start of each async test before calling EnvVarGuard::set, holding it for the duration of the test so the env changes are effectively serialized, or alternatively mark the tests serial using a test-serialization helper (like the serial_test crate) so EnvVarGuard::set is never executed concurrently.modules/cerebro/tests/storage_config_test.rs (2)
150-169:⚠️ Potential issue | 🟡 MinorMissing
ENV_LOCKacquisition before env var mutation.
fallback_reports_active_modesetsCEREBRO_TEST_FAIL_STORAGEat line 157 without holdingENV_LOCK, unlike the other tests that mutate environment variables. This can cause race conditions in parallel test execution.Suggested fix
#[tokio::test] async fn fallback_reports_active_mode() { let buffer = Arc::new(Mutex::new(Vec::new())); let writer = BufferWriter(buffer.clone()); let subscriber = tracing_subscriber::fmt().with_writer(writer).finish(); let _guard = tracing::subscriber::set_default(subscriber); + let _lock = ENV_LOCK.lock().expect("env lock"); let _env = EnvVarGuard::set("CEREBRO_TEST_FAIL_STORAGE", "1");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/storage_config_test.rs` around lines 150 - 169, The test fallback_reports_active_mode mutates the environment via EnvVarGuard without acquiring the global ENV_LOCK, risking race conditions; update the test to acquire the ENV_LOCK before creating the EnvVarGuard (e.g., obtain a guard from ENV_LOCK.lock() and hold it for the duration of the test's env mutation), so the EnvVarGuard usage in fallback_reports_active_mode is protected the same way as other tests that call storage_from_config and construct CerebroConfig.
137-148: 🧹 Nitpick | 🔵 TrivialSilently ignoring lock poisoning masks prior test panics.
BufferGuard::writeusesif let Ok(mut guard)which silently ignores a poisoned mutex. In tests, a poisoned lock indicates a prior panic that should propagate failure.Suggested fix
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> { - if let Ok(mut guard) = self.0.lock() { - guard.extend_from_slice(buf); - } + let mut guard = self.0.lock().expect("buffer lock poisoned"); + guard.extend_from_slice(buf); Ok(buf.len()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/storage_config_test.rs` around lines 137 - 148, The write implementation on BufferGuard currently ignores a poisoned mutex by using `if let Ok(mut guard) = self.0.lock()`, masking prior panics; change it to explicitly unwrap the lock so a PoisonError panics and fails the test (e.g. `let mut guard = self.0.lock().unwrap(); guard.extend_from_slice(buf);`) inside the `impl std::io::Write for BufferGuard` `write` method.modules/cerebro/src/migration/legacy.rs (1)
106-111: 🧹 Nitpick | 🔵 TrivialUse idiomatic
is_none()instead of negatedis_some().Line 106 uses
!observation.get("content").is_some()which is less idiomatic. Preferis_none()for clarity.Suggested fix
- if !observation.get("content").is_some() { + if observation.get("content").is_none() { tracing::warn!( memory_id = %memory_id, "observation missing 'content' field, allowing with empty content" ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/migration/legacy.rs` around lines 106 - 111, Replace the negated is_some() check with is_none() for clarity: change the condition that currently reads using observation.get("content").is_some() negated to observation.get("content").is_none(), keeping the surrounding logic and the tracing::warn call (memory_id and message) intact so behavior doesn't change but the code becomes idiomatic.modules/cerebro/src/tui/mod.rs (1)
203-213: 🧹 Nitpick | 🔵 TrivialRuntime event errors still mapped to
InitFailed.
crossterm::event::poll(line 204) andevent::read(line 207) errors occur during the main loop, not initialization. UsingTuiError::InitFailedis semantically misleading. Consider adding aTuiError::EventErrorvariant.Suggested change
#[derive(Debug, thiserror::Error)] pub enum TuiError { #[error("tui is disabled at compile time")] FeatureDisabled, #[error("tui initialization failed: {0}")] InitFailed(String), #[error("tui render failed: {0}")] RenderFailed(String), + #[error("tui event error: {0}")] + EventError(String), #[error("tui task failed to start: {0}")] TaskStart(String), } // Then in run_tui: - if crossterm::event::poll(Duration::from_millis(50)) - .map_err(|err| TuiError::InitFailed(err.to_string()))? + if crossterm::event::poll(Duration::from_millis(50)) + .map_err(|err| TuiError::EventError(err.to_string()))?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tui/mod.rs` around lines 203 - 213, The code maps runtime event errors from crossterm::event::poll and crossterm::event::read to TuiError::InitFailed, which is misleading; add a new TuiError::EventError variant and update the error mappings in the loop so poll(...) .map_err(|err| TuiError::EventError(err.to_string()))? and event::read() .map_err(|err| TuiError::EventError(err.to_string()))? are used instead of InitFailed, leaving initialization-related uses of TuiError::InitFailed unchanged; update any match uses of TuiError to handle the new EventError if necessary and keep app.on_key(key) logic intact.modules/cerebro/src/tui/redaction.rs (1)
100-108: 🧹 Nitpick | 🔵 TrivialAllowlist not propagated to nested structures is secure but may over-redact.
Lines 100 and 107 pass
Nonefor the allowlist when recursing into nested objects/arrays. This means all nested fields are redacted regardless of the top-level allowlist. This is the secure default (deny-by-default), but may over-redact nested data that should be visible.If this is intentional, add a brief doc comment explaining the design. If nested allowlisting is needed, propagate the allowlist:
To propagate allowlist (if needed)
- output.insert(key.clone(), self.redact_value(value, None)); + output.insert(key.clone(), self.redact_value(value, allowlist)); // ... - .map(|value| self.redact_value(value, None)) + .map(|value| self.redact_value(value, allowlist))Based on learnings: "Do not silently weaken security policy" - current behavior is secure, consider documenting rather than changing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/src/tui/redaction.rs` around lines 100 - 108, The recursion into nested structures currently calls redact_value(value, None) from the Value::Object and Value::Array arms which drops any top-level allowlist and causes deny-by-default redaction for all nested fields; either (A) explicitly propagate the current allowlist into recursive calls (e.g., pass the same allowlist reference/clone into redact_value instead of None) so nested keys respect the top-level allowlist, or (B) if the deny-by-default behavior is intentional, add a brief doc comment on the redact_value function and the Value::Object/Value::Array branches explaining that nested allowlists are not propagated and that this is a deliberate security decision. Ensure you reference the redact_value calls in the Value::Object/Value::Array handling when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md`:
- Line 123: Docs refer to a non-existent "cerebro serve" CLI subcommand while
the repo only builds a "cerebro-serve" binary; either update the docs to
instruct users to run "cerebro-serve --tui" everywhere (replace the "cerebro
serve" occurrences) or add a wrapper/alias binary named "cerebro" (implement
src/bin/cerebro.rs or adjust the Cargo.toml [[bin]] entries to expose a
"cerebro" binary that delegates to the main code) and ensure the Cargo.toml
binaries and src/main.rs align with the chosen CLI name so the documented
command works.
In `@modules/cerebro/src/config.rs`:
- Around line 372-377: The env_flag function currently only recognizes truthy
values and cannot distinguish an explicit false from an unset variable; change
env_flag to return Option<bool> (None when the env var is not set or
unrecognized, Some(true) for "1"/"true"/"yes"/"on", Some(false) for
"0"/"false"/"no"/"off") so callers can detect explicit overrides; update all
call sites that use env_flag to treat Some(value) as an override of config (use
the boolean) and None to fall back to the config value; refer to the env_flag
function and the CEREBRO_TUI_ENABLED env var when making these changes.
In `@modules/cerebro/src/main.rs`:
- Around line 45-46: The TUI task handle returned in the
TuiLaunch::Started(_handle) branch is being dropped immediately so the spawned
TUI isn't awaited on shutdown; modify main.rs to capture and store that handle
(e.g., in a variable like tui_handle or Option<JoinHandle<...>>) when matching
TuiLaunch::Started, move it out of the spawn context so it lives until shutdown,
and after axum's server shutdown awaits the stored handle (join/await) to ensure
the TUI completes its graceful shutdown instead of being dropped immediately.
- Around line 28-31: The network listener validation is performed after the TCP
bind, so when validate_no_network_listeners() fails the port is already bound;
move the call to cerebro::tui::validate_no_network_listeners() to run before the
TcpListener::bind() when config.tui.enabled is true so the function returns
early on invalid configuration; update the block that checks config.tui.enabled
to call validate_no_network_listeners() first and only proceed to
TcpListener::bind() if validation succeeds.
In `@modules/cerebro/src/migration/legacy.rs`:
- Around line 54-67: The function read_legacy_export currently uses blocking
std::fs::read_to_string which can block an async runtime; change
read_legacy_export to an async function (async fn read_legacy_export(path:
&Path) -> Result<LegacyExport, CerebroError>) and replace
std::fs::read_to_string with tokio::fs::read_to_string(path).await, preserving
the existing error mapping to CerebroError::Storage and
CerebroError::Validation, and update any callers to await the function (or
alternatively provide a new async variant and keep the sync one delegating to
spawn_blocking if you must preserve sync callers).
In `@modules/cerebro/src/migration/mod.rs`:
- Around line 68-73: The MigrationReport currently returns only the `actual`
collections (variable `actual`) in its `collections` field even when `status` ==
`Mismatch`, which hides what was expected; update the code and the
`MigrationReport` type (struct `MigrationReport`) so the report includes both
`expected` and `actual` collections (e.g., add an `expected` field or replace
`collections` with an `expected_and_actual` pair) and populate both fields where
the report is constructed (the block returning `Ok(MigrationReport { source:
..., target: ..., collections: actual, status, })`) so callers can see both sets
when `status` is `Mismatch`.
In `@modules/cerebro/src/tui/redaction.rs`:
- Around line 19-20: TOKEN_RE is too permissive and may redact benign long
identifiers; either tighten the regex or document the intentional trade-off.
Update the static TOKEN_RE (the Lazy<Regex> initializer) to a more specific
pattern that reduces false positives (for example require mixed character
classes, known token prefixes/suffixes, or anchor patterns for the token types
you intend to catch), or add a clear comment above TOKEN_RE explaining why the
broad [A-Za-z0-9_-]{24,} pattern is required and acceptable for security. Ensure
the chosen approach is applied to the TOKEN_RE definition and its comment to
make the intent explicit.
In `@modules/cerebro/tests/migration_legacy_test.rs`:
- Around line 14-24: The test normalizes_memory_ids_and_sorting is using a
hardcoded PathBuf for the fixture; replace that with the fixture_path() helper
to ensure correct test path resolution: update the test to call
fixture_path("legacy/legacy_export.json") (or appropriate relative arg) instead
of PathBuf::from("tests/fixtures/legacy/legacy_export.json") so
read_legacy_export(&path) receives the correct PathBuf; keep the rest of the
test assertions and calls to read_legacy_export and normalize_export unchanged.
In `@modules/cerebro/tests/tui_non_blocking_tests.rs`:
- Around line 28-53: The test mutates CEREBRO_TUI_HEADLESS directly which can
leak on panic and race in parallel runs; replace the manual
std::env::set_var/remove_var with the existing RAII helper used in
storage_config_test.rs (EnvVarGuard) so the env var is restored on drop and the
helper's test lock is acquired to prevent races. Specifically, at the top of the
test create an EnvVarGuard for "CEREBRO_TUI_HEADLESS" (e.g.,
EnvVarGuard::new("CEREBRO_TUI_HEADLESS", Some("1"))) before calling
start_tui_task/start_tui_task(tui_config,...), keep the guard alive for the
duration of the test scope (so it drops after handle.join()), and remove the
explicit std::env::remove_var and std::env::set_var calls; ensure you import
EnvVarGuard from the same module as storage_config_test.rs.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.md`:
- Around line 21-23: Update the proposal text that currently lists "TUI
enhancements or implementation (explicitly optional for this phase)" to reflect
that this PR delivers a feature‑gated TUI: either change the phrasing to "TUI
enhancements or implementation (delivered as a feature‑gated optional add‑on in
this PR)" or add a short note beneath the scope list stating that the PR expands
scope by including an optional, feature‑gated TUI implementation; reference the
proposal.md section containing the scope list so reviewers can see the changed
wording and whether this is an accepted scope expansion.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.md`:
- Around line 69-70: Update the migration requirement sentence that currently
reads "The migration tooling MUST provide validation that verifies import
completeness (at minimum, record counts and schema compatibility)" to explicitly
include document checksum validation: state that the tooling MUST also verify
canonical JSON document checksums to ensure import completeness and integrity
(in addition to record counts and schema compatibility). Reference "document
checksum validation" and "canonical JSON checksum" in the requirement so the
spec matches the implemented validation semantics.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md`:
- Line 43: The document contains contradictory coverage statements: the line
containing "Coverage: ✅ Passed (threshold: 60%)" conflicts with the later
sentence that "coverage reporting was not performed"; pick the single correct
outcome and make both statements identical and auditable (either change the
"Coverage: ✅ Passed (threshold: 60%)" line to reflect that reporting was not
performed, or replace the later "coverage reporting was not performed" sentence
with a short audit statement confirming the pass and linking to the coverage
artifact). Update both occurrences so the report consistently states the same
coverage result and include a brief reference to the evidence/artifact ID used
to decide the outcome.
In `@PRD-English.md`:
- Around line 1-43: Add an explicit "Translation status" note to PRD-English.md
indicating English is complete and whether a Spanish counterpart exists or is
pending (e.g., "Translation status: English — complete; Spanish — pending / see
PRD-Spanish.md"), and if an ES file will be added, create or link PRD-Spanish.md
and update the note to point to it; place the short note near the top under the
title or at the end so readers immediately see EN/ES parity status.
- Around line 12-39: Update the PRD to explicitly document the new Cerebro
runtime behaviors: add bullets under "2. Key Features" for "Embedded SurrealDB
migration flow", "Remote Surreal safety checks", "Storage fallback policy (local
-> remote tiers)", and "TUI redaction and limits"; update "4. Security and
Performance Principles" to reference the remote Surreal safety checks and TUI
redaction/limits as operational security controls; and extend "6. Integration
and Testing" with test requirements for migration flows, safety-check
regressions, fallback-policy scenarios, and TUI redaction/limit tests so the doc
matches shipped behavior.
---
Duplicate comments:
In `@modules/cerebro/src/migration/legacy.rs`:
- Around line 106-111: Replace the negated is_some() check with is_none() for
clarity: change the condition that currently reads using
observation.get("content").is_some() negated to
observation.get("content").is_none(), keeping the surrounding logic and the
tracing::warn call (memory_id and message) intact so behavior doesn't change but
the code becomes idiomatic.
In `@modules/cerebro/src/tools.rs`:
- Around line 233-293: The function extract_safe_args currently takes payload:
&Value and calls payload.clone() in every match arm before
serde_json::from_value, causing unnecessary allocations; change the signature to
extract_safe_args(&self, tool: &str, payload: Value) and remove the .clone()
calls so serde_json::from_value(payload) consumes the value directly, or
alternately only clone in arms that need it; update all references to
payload.clone() in extract_safe_args (and any callers) to pass ownership of the
Value instead of a reference.
- Around line 39-42: The mem_suggest_topic_key allowlist currently exposes user
text via the "seed" arg; update the mapping for the "mem_suggest_topic_key" case
in tools.rs so allowed_arg_fields only contains "scope" (remove "seed"), leaving
allowed_output_fields unchanged; locate the match/constructor where
mem_suggest_topic_key is created and modify the allowed_arg_fields slice to
["scope"] to eliminate logging of user-provided seed text.
- Around line 273-280: In the "mem_suggest_topic_key" branch of
extract_safe_args, stop exposing the user-provided seed: parse the payload into
ToolInput<MemSuggestTopicKeyRequest> as before (using payload.clone()) but build
the returned safe-args JSON without the "seed" field (only include "scope" or
any non-sensitive fields), so remove "seed" from the json! output to align with
allowed_arg_fields and the redaction policy.
In `@modules/cerebro/src/tui/mod.rs`:
- Around line 203-213: The code maps runtime event errors from
crossterm::event::poll and crossterm::event::read to TuiError::InitFailed, which
is misleading; add a new TuiError::EventError variant and update the error
mappings in the loop so poll(...) .map_err(|err|
TuiError::EventError(err.to_string()))? and event::read() .map_err(|err|
TuiError::EventError(err.to_string()))? are used instead of InitFailed, leaving
initialization-related uses of TuiError::InitFailed unchanged; update any match
uses of TuiError to handle the new EventError if necessary and keep
app.on_key(key) logic intact.
In `@modules/cerebro/src/tui/redaction.rs`:
- Around line 100-108: The recursion into nested structures currently calls
redact_value(value, None) from the Value::Object and Value::Array arms which
drops any top-level allowlist and causes deny-by-default redaction for all
nested fields; either (A) explicitly propagate the current allowlist into
recursive calls (e.g., pass the same allowlist reference/clone into redact_value
instead of None) so nested keys respect the top-level allowlist, or (B) if the
deny-by-default behavior is intentional, add a brief doc comment on the
redact_value function and the Value::Object/Value::Array branches explaining
that nested allowlists are not propagated and that this is a deliberate security
decision. Ensure you reference the redact_value calls in the
Value::Object/Value::Array handling when applying the change.
In `@modules/cerebro/src/tui/views/mod.rs`:
- Around line 32-35: The vertical layout's first constraint
(Constraint::Length(2)) is too small for the Tabs widget which uses Borders::ALL
and padding; update the constraints passed to Layout::default().constraints(...)
(the array containing Constraint::Length(2), Constraint::Length(1)) to use at
least Constraint::Length(5) for the tabs area so the Tabs widget (with
Borders::ALL and internal padding) has enough rows to render correctly.
In `@modules/cerebro/tests/cli_migration_test.rs`:
- Around line 63-65: The test currently optionally mutates the JSON with
json.get_mut("memory").and_then(Value::as_array_mut) { array.pop(); } which can
silently skip the mutation; make the mismatch mutation mandatory by asserting
that the "memory" field exists and is a non-empty array, then perform the
mutation (e.g., obtain a mutable array via
json.get_mut("memory").and_then(Value::as_array_mut).expect(...) and call
pop()), so the test fails fast if the precondition isn't met and the mutation
always occurs.
In `@modules/cerebro/tests/migration_legacy_test.rs`:
- Around line 4-12: The test parses_legacy_export_fixture uses a relative
PathBuf which is cwd-dependent; change the PathBuf construction to use the crate
root via env!("CARGO_MANIFEST_DIR") and join the fixture path so the test loads
deterministically. Locate the test function parses_legacy_export_fixture and
replace the PathBuf::from("tests/fixtures/legacy/legacy_export.json") call with
a PathBuf built from env!("CARGO_MANIFEST_DIR") joined with
"tests/fixtures/legacy/legacy_export.json" so read_legacy_export(&path) always
finds the fixture regardless of working directory.
In `@modules/cerebro/tests/storage_config_test.rs`:
- Around line 150-169: The test fallback_reports_active_mode mutates the
environment via EnvVarGuard without acquiring the global ENV_LOCK, risking race
conditions; update the test to acquire the ENV_LOCK before creating the
EnvVarGuard (e.g., obtain a guard from ENV_LOCK.lock() and hold it for the
duration of the test's env mutation), so the EnvVarGuard usage in
fallback_reports_active_mode is protected the same way as other tests that call
storage_from_config and construct CerebroConfig.
- Around line 137-148: The write implementation on BufferGuard currently ignores
a poisoned mutex by using `if let Ok(mut guard) = self.0.lock()`, masking prior
panics; change it to explicitly unwrap the lock so a PoisonError panics and
fails the test (e.g. `let mut guard = self.0.lock().unwrap();
guard.extend_from_slice(buf);`) inside the `impl std::io::Write for BufferGuard`
`write` method.
In `@modules/cerebro/tests/tui_shutdown_tests.rs`:
- Around line 34-60: The tests mutate process-global env vars via
EnvVarGuard::set (e.g., in the test functions that set "CEREBRO_TUI_HEADLESS"
and "CEREBRO_TUI_TEST_CRASH") and must serialize those mutations to avoid
cross-test interference; fix by introducing a shared global async lock (e.g., a
static Lazy<tokio::sync::Mutex<()>> or similar) and acquire the lock at the
start of each async test before calling EnvVarGuard::set, holding it for the
duration of the test so the env changes are effectively serialized, or
alternatively mark the tests serial using a test-serialization helper (like the
serial_test crate) so EnvVarGuard::set is never executed concurrently.
In `@modules/cerebro/tests/tui_toggle_tests.rs`:
- Line 72: The test currently awaits handle.join().await without a timeout,
which can hang; wrap the join in a bounded timeout (e.g., using
tokio::time::timeout with a suitable Duration) and assert that the timeout did
not elapse before unwrapping the join result; update the await call on
handle.join() in tui_toggle_tests.rs to use tokio::time::timeout(...,
handle.join()).await and fail the test if the timeout occurs while still
preserving the existing expect("tui join should succeed") on the join result.
In
`@openspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.md`:
- Around line 8-90: The document jumps heading levels (H1 to H3) and lacks a
blank line after the "Verdict" heading causing MD001/MD022 warnings; update the
headings under the top-level title so section headings like "Completeness",
"Build & Tests Execution", "Spec Compliance Matrix", "Correctness (Static —
Structural Evidence)", "Coherence (Design)", "Issues Found", and "Verdict" use
consistent H2 (i.e., "##") or H3 levels as appropriate (e.g., change "###
Completeness" to "## Completeness" to avoid skipping levels) and insert a single
blank line after the "Verdict" heading before the verdict text to ensure proper
spacing. Ensure changes touch the heading lines with the exact headings
mentioned above so linters MD001/MD022 pass.
In `@PRD-English.md`:
- Around line 3-37: The file has multiple headings that violate MD022 (no blank
line after a heading); for each heading like "## 1. Introduction and
Objectives", "### Objectives", "## 2. Key Features", "## 3. Architecture
Overview", "## 4. Security and Performance Principles", "## 5. Development
Workflow", and "## 6. Integration and Testing", insert a single blank line
immediately after the heading line so every heading is followed by an empty line
before the next content paragraph or list; update each heading occurrence
accordingly to resolve the MD022 lint error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23faf89d-7966-49e2-bc2b-d958c32d50c9
⛔ Files ignored due to path filters (2)
clients/agent-runtime/Cargo.lockis excluded by!**/*.lockmodules/cerebro/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
PRD-Corvus.mdPRD-English.mdREADME.mdclients/web/apps/docs/src/content/docs/guides/cerebro/migration.mdlychee.tomlmodules/cerebro/Cargo.tomlmodules/cerebro/src/config.rsmodules/cerebro/src/main.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/src/server.rsmodules/cerebro/src/storage/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/tui/views/live_logs.rsmodules/cerebro/src/tui/views/memory_explorer.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/embedded_storage_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/migration_report_test.rsmodules/cerebro/tests/migration_workflow_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_event_emission_tests.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/tui_toggle_tests.rsopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/design.mdopenspec/changes/archive/2026-03-17-2026-03-17-cerebro-spec-refresh/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/tasks.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/design.mdopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-tui-phase2-optional-surface/verify-report.mdopenspec/specs/cerebro/prompt_template.mdopenspec/specs/cerebro/spec.md
| cerebro serve --tui | ||
| ``` | ||
|
|
||
| Enable via environment for the `cerebro-serve` binary: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Declared binaries in modules/cerebro/Cargo.toml =="
rg -n '^\[\[bin\]\]|^name\s*=' modules/cerebro/Cargo.toml -C2
echo
echo "== CLI entrypoint files =="
fd . modules/cerebro/src/bin
echo
echo "== TUI env var and command mentions in docs =="
rg -n 'cerebro-serve|cerebro serve|CEREBRO_TUI_ENABLED' \
clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md -C2
echo
echo "== TUI env var usage in Cerebro code =="
rg -n 'CEREBRO_TUI_ENABLED|tui\.enabled|--tui' modules/cerebro/src -C2Repository: dallay/corvus
Length of output: 518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full context around line 123 in migration.md =="
sed -n '115,135p' clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md
echo
echo "== Full TUI env var and command search in docs =="
rg -n 'cerebro-serve|cerebro serve|CEREBRO_TUI_ENABLED' \
clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md
echo
echo "== Full TUI env var and command usage in code =="
rg -n 'CEREBRO_TUI_ENABLED|tui.*enabled|--tui' modules/cerebro/src
echo
echo "== Binary purpose inspection: main.rs vs cerebro.rs =="
echo "--- main.rs (cerebro-serve) first lines ---"
head -20 modules/cerebro/src/main.rs
echo
echo "--- cerebro.rs (cerebro) first lines ---"
head -20 modules/cerebro/src/bin/cerebro.rsRepository: dallay/corvus
Length of output: 2005
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Actual contents of modules/cerebro/src =="
find modules/cerebro/src -type f -name "*.rs" | head -20
echo
echo "== Check if src/bin directory exists =="
ls -la modules/cerebro/src/bin 2>&1 || echo "src/bin does not exist"
echo
echo "== Double-check Cargo.toml bin sections =="
cat modules/cerebro/Cargo.toml | grep -A3 "\[\[bin\]\]"
echo
echo "== Verify main.rs exists and is the server =="
head -5 modules/cerebro/src/main.rsRepository: dallay/corvus
Length of output: 1345
Fix CLI invocation inconsistency: docs reference cerebro serve but only cerebro-serve binary exists.
Line 120 documents cerebro serve --tui, but Cargo.toml declares only one buildable binary: cerebro-serve (from src/main.rs). The cerebro binary referenced in Cargo.toml points to a non-existent src/bin/cerebro.rs. Verify whether cerebro serve is a wrapper/alias or if the docs should use cerebro-serve --tui directly. This inconsistency will cause operators following the docs to fail.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/docs/src/content/docs/guides/cerebro/migration.md` at line
123, Docs refer to a non-existent "cerebro serve" CLI subcommand while the repo
only builds a "cerebro-serve" binary; either update the docs to instruct users
to run "cerebro-serve --tui" everywhere (replace the "cerebro serve"
occurrences) or add a wrapper/alias binary named "cerebro" (implement
src/bin/cerebro.rs or adjust the Cargo.toml [[bin]] entries to expose a
"cerebro" binary that delegates to the main code) and ensure the Cargo.toml
binaries and src/main.rs align with the chosen CLI name so the documented
command works.
| fn env_flag(key: &str) -> bool { | ||
| std::env::var(key) | ||
| .ok() | ||
| .map(|value| value.trim().to_ascii_lowercase()) | ||
| .is_some_and(|value| matches!(value.as_str(), "1" | "true" | "yes" | "on")) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
env_flag only handles truthy values.
Setting CEREBRO_TUI_ENABLED=0 or CEREBRO_TUI_ENABLED=false won't explicitly disable TUI if it's enabled in the config file. The function only checks for truthy values, so explicit disable would require unsetting the var entirely.
This is acceptable if the intent is "env can only enable, not disable," but consider documenting this behavior or adding falsy value handling if explicit override is desired.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/config.rs` around lines 372 - 377, The env_flag function
currently only recognizes truthy values and cannot distinguish an explicit false
from an unset variable; change env_flag to return Option<bool> (None when the
env var is not set or unrecognized, Some(true) for "1"/"true"/"yes"/"on",
Some(false) for "0"/"false"/"no"/"off") so callers can detect explicit
overrides; update all call sites that use env_flag to treat Some(value) as an
override of config (use the boolean) and None to fall back to the config value;
refer to the env_flag function and the CEREBRO_TUI_ENABLED env var when making
these changes.
| if config.tui.enabled { | ||
| if let Err(err) = cerebro::tui::validate_no_network_listeners() { | ||
| return Err(anyhow!("tui validation failed: {err}")); | ||
| } |
There was a problem hiding this comment.
Network listener validation runs after TCP bind.
validate_no_network_listeners() is called at line 29, but the TcpListener::bind() already executed at line 22. If validation fails, the port is already bound. Move validation before binding to fail fast.
Suggested fix
let config = CerebroConfig::load(config_path.as_deref())?.apply_env_overrides();
+ if config.tui.enabled {
+ if let Err(err) = cerebro::tui::validate_no_network_listeners() {
+ return Err(anyhow!("tui validation failed: {err}"));
+ }
+ }
let addr = config.bind_addr();
let service = Arc::new(CerebroService::from_config(config.clone()).await?);
let listener = TcpListener::bind(&addr).await?;
tracing::info!(%addr, "Cerebro MCP listening");
let (shutdown_tx, shutdown_rx) = watch::channel(false);
tokio::spawn(shutdown_signal(shutdown_tx));
if config.tui.enabled {
- if let Err(err) = cerebro::tui::validate_no_network_listeners() {
- return Err(anyhow!("tui validation failed: {err}"));
- }
let tui_config = config.tui.clone();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/main.rs` around lines 28 - 31, The network listener
validation is performed after the TCP bind, so when
validate_no_network_listeners() fails the port is already bound; move the call
to cerebro::tui::validate_no_network_listeners() to run before the
TcpListener::bind() when config.tui.enabled is true so the function returns
early on invalid configuration; update the block that checks config.tui.enabled
to call validate_no_network_listeners() first and only proceed to
TcpListener::bind() if validation succeeds.
| Ok(TuiLaunch::Started(_handle)) => { | ||
| tracing::info!("tui started"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
TUI handle dropped without awaiting graceful shutdown.
_handle at line 45 is immediately dropped, so the TUI task isn't awaited during shutdown. The TUI will terminate abruptly when the process exits rather than completing its shutdown sequence. Consider storing the handle and awaiting it after axum shuts down.
Suggested approach
+ let mut tui_handle: Option<cerebro::tui::TuiHandle> = None;
if config.tui.enabled {
// ... validation ...
tokio::spawn(async move {
match start_tui_task(...).await {
- Ok(TuiLaunch::Started(_handle)) => {
+ Ok(TuiLaunch::Started(handle)) => {
tracing::info!("tui started");
+ // Store handle for later await
}
// ...
}
});
}
axum::serve(listener, service.router())
.with_graceful_shutdown(wait_for_shutdown(shutdown_rx))
.await?;
+
+ if let Some(handle) = tui_handle {
+ let _ = handle.join().await;
+ }
Ok(())Note: This requires restructuring to move the handle out of the spawned task.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/main.rs` around lines 45 - 46, The TUI task handle
returned in the TuiLaunch::Started(_handle) branch is being dropped immediately
so the spawned TUI isn't awaited on shutdown; modify main.rs to capture and
store that handle (e.g., in a variable like tui_handle or
Option<JoinHandle<...>>) when matching TuiLaunch::Started, move it out of the
spawn context so it lives until shutdown, and after axum's server shutdown
awaits the stored handle (join/await) to ensure the TUI completes its graceful
shutdown instead of being dropped immediately.
| pub fn read_legacy_export(path: &Path) -> Result<LegacyExport, CerebroError> { | ||
| let data = std::fs::read_to_string(path).map_err(|err| { | ||
| CerebroError::Storage(format!( | ||
| "failed to read legacy export {}: {err}", | ||
| path.display() | ||
| )) | ||
| })?; | ||
| serde_json::from_str(&data).map_err(|err| { | ||
| CerebroError::Validation(format!( | ||
| "failed to parse legacy export {}: {err}", | ||
| path.display() | ||
| )) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Blocking file I/O in potentially async context.
std::fs::read_to_string is blocking. If this function is called from an async runtime, it could block the executor. For migration tooling this is likely acceptable, but consider tokio::fs::read_to_string if latency matters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/migration/legacy.rs` around lines 54 - 67, The function
read_legacy_export currently uses blocking std::fs::read_to_string which can
block an async runtime; change read_legacy_export to an async function (async fn
read_legacy_export(path: &Path) -> Result<LegacyExport, CerebroError>) and
replace std::fs::read_to_string with tokio::fs::read_to_string(path).await,
preserving the existing error mapping to CerebroError::Storage and
CerebroError::Validation, and update any callers to await the function (or
alternatively provide a new async variant and keep the sync one delegating to
spawn_blocking if you must preserve sync callers).
| - TUI enhancements or implementation (explicitly optional for this phase). | ||
| - Changes to the MCP tool surface beyond what is necessary to support migration tooling. | ||
| - Any new runtime-local SurrealDB backend (remains removed per current Cerebro spec). |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Scope clarification: TUI is included in this PR.
The proposal states TUI is "explicitly optional for this phase" and out of scope, but this PR delivers a feature-gated TUI implementation. Consider updating the document to reflect that TUI was delivered as an optional add-on, or note this as a scope expansion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/proposal.md`
around lines 21 - 23, Update the proposal text that currently lists "TUI
enhancements or implementation (explicitly optional for this phase)" to reflect
that this PR delivers a feature‑gated TUI: either change the phrasing to "TUI
enhancements or implementation (delivered as a feature‑gated optional add‑on in
this PR)" or add a short note beneath the scope list stating that the PR expands
scope by including an optional, feature‑gated TUI implementation; reference the
proposal.md section containing the scope list so reviewers can see the changed
wording and whether this is an accepted scope expansion.
| The migration tooling MUST provide validation that verifies import completeness (at minimum, record | ||
| counts and schema compatibility). |
There was a problem hiding this comment.
Document checksum validation explicitly in the migration requirement.
Line 69–70 only calls out counts/schema compatibility. The PR introduces canonical JSON checksum validation; the requirement should state that explicitly so the spec matches implemented validation semantics.
Suggested patch
-The migration tooling MUST provide validation that verifies import completeness (at minimum, record
-counts and schema compatibility).
+The migration tooling MUST provide validation that verifies import completeness (at minimum, record
+counts, canonical JSON checksum consistency, and schema compatibility).As per coding guidelines, **/*.{md,mdx}: "Verify technical accuracy and that docs stay aligned with code changes."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The migration tooling MUST provide validation that verifies import completeness (at minimum, record | |
| counts and schema compatibility). | |
| The migration tooling MUST provide validation that verifies import completeness (at minimum, record | |
| counts, canonical JSON checksum consistency, and schema compatibility). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/specs/cerebro/spec.md`
around lines 69 - 70, Update the migration requirement sentence that currently
reads "The migration tooling MUST provide validation that verifies import
completeness (at minimum, record counts and schema compatibility)" to explicitly
include document checksum validation: state that the tooling MUST also verify
canonical JSON document checksums to ensure import completeness and integrity
(in addition to record counts and schema compatibility). Reference "document
checksum validation" and "canonical JSON checksum" in the requirement so the
spec matches the implemented validation semantics.
| # Product Requirements Document (PRD) | ||
|
|
||
| ## 1. Introduction and Objectives | ||
| The Corvus project is a Gradle-based multi-module system written in Kotlin, designed to support a variety of applications including Android, iOS, and Compose desktop apps. It emphasizes centralized build configurations, custom plugins, and version catalogs to ensure consistency and maintainability. The project also integrates a high-performance Rust-based memory system (Cerebro) for AI agents. | ||
|
|
||
| ### Objectives | ||
| - Provide a modular and scalable architecture for multi-platform development. | ||
| - Ensure security and performance as primary principles. | ||
| - Facilitate seamless integration with AI agents and memory systems. | ||
| - Support Test-Driven Development (TDD) and maintain high code quality. | ||
|
|
||
| ## 2. Key Features | ||
| - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. | ||
| - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. | ||
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. | ||
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | ||
| - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. | ||
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | ||
|
|
||
| ## 3. Architecture Overview | ||
| The project is structured into the following components: | ||
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | ||
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | ||
| - **Gradle**: Custom build logic and version catalogs. | ||
| - **Docs**: Comprehensive documentation for developers. | ||
|
|
||
| ## 4. Security and Performance Principles | ||
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | ||
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | ||
|
|
||
| ## 5. Development Workflow | ||
| - Follow TDD: Red -> Green -> Refactor. | ||
| - Use Makefile commands for streamlined development. | ||
| - Maintain code quality with Spotless, Detekt, and other tools. | ||
|
|
||
| ## 6. Integration and Testing | ||
| - Integrate with AI agents via the MCP JSON-RPC protocol. | ||
| - Test modules independently and as part of the whole system. | ||
| - Ensure high test coverage and adherence to coding standards. | ||
|
|
||
| --- | ||
|
|
||
| This document serves as a high-level overview of the Corvus project, outlining its goals, features, and architecture. The next step is to adapt this PRD to the specific context and requirements of the project. |
There was a problem hiding this comment.
Add explicit EN/ES translation status
This is user-facing documentation, but there is no Spanish parity file or an explicit note that ES is pending. Please add a short “Translation status” note (or link the ES counterpart) to avoid ambiguity.
As per coding guidelines, "For user-facing docs, check EN/ES parity or explicitly note pending translation gaps."
🧰 Tools
🪛 LanguageTool
[style] ~4-~4: Consider using a more concise synonym.
Context: ... written in Kotlin, designed to support a variety of applications including Android, iOS, an...
(A_VARIETY_OF)
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD-English.md` around lines 1 - 43, Add an explicit "Translation status"
note to PRD-English.md indicating English is complete and whether a Spanish
counterpart exists or is pending (e.g., "Translation status: English — complete;
Spanish — pending / see PRD-Spanish.md"), and if an ES file will be added,
create or link PRD-Spanish.md and update the note to point to it; place the
short note near the top under the title or at the end so readers immediately see
EN/ES parity status.
| ## 2. Key Features | ||
| - **Multi-Platform Support**: Modules for Android, iOS, and Compose desktop applications. | ||
| - **Cerebro Memory System**: A Rust-based memory module with SurrealDB for multi-model storage. | ||
| - **Centralized Build Configurations**: Custom Gradle plugins and version catalogs. | ||
| - **Security-First Design**: Emphasis on safe defaults, data validation, and least privilege. | ||
| - **Performance Optimization**: Lazy initialization, efficient algorithms, and profiling. | ||
| - **Developer Tools**: Makefile commands for build, test, and maintenance tasks. | ||
|
|
||
| ## 3. Architecture Overview | ||
| The project is structured into the following components: | ||
| - **Apps**: Android, iOS, and Compose modules for platform-specific implementations. | ||
| - **Modules**: Shared Kotlin Multiplatform core and Rust-based Cerebro memory system. | ||
| - **Gradle**: Custom build logic and version catalogs. | ||
| - **Docs**: Comprehensive documentation for developers. | ||
|
|
||
| ## 4. Security and Performance Principles | ||
| - **Security**: Validate and sanitize all data, use parameterized queries, and follow the principle of least privilege. | ||
| - **Performance**: Optimize for algorithmic efficiency, avoid unnecessary allocations, and measure before optimizing. | ||
|
|
||
| ## 5. Development Workflow | ||
| - Follow TDD: Red -> Green -> Refactor. | ||
| - Use Makefile commands for streamlined development. | ||
| - Maintain code quality with Spotless, Detekt, and other tools. | ||
|
|
||
| ## 6. Integration and Testing | ||
| - Integrate with AI agents via the MCP JSON-RPC protocol. | ||
| - Test modules independently and as part of the whole system. | ||
| - Ensure high test coverage and adherence to coding standards. |
There was a problem hiding this comment.
Doc scope is not aligned with this PR’s behavioral changes
This PRD omits the newly introduced Cerebro behaviors that matter operationally/security-wise (embedded SurrealDB migration flow, remote Surreal safety checks, storage fallback policy, and TUI redaction/limits). Please add these explicitly under Features/Security/Testing so the document matches shipped behavior.
As per coding guidelines, "**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@PRD-English.md` around lines 12 - 39, Update the PRD to explicitly document
the new Cerebro runtime behaviors: add bullets under "2. Key Features" for
"Embedded SurrealDB migration flow", "Remote Surreal safety checks", "Storage
fallback policy (local -> remote tiers)", and "TUI redaction and limits"; update
"4. Security and Performance Principles" to reference the remote Surreal safety
checks and TUI redaction/limits as operational security controls; and extend "6.
Integration and Testing" with test requirements for migration flows,
safety-check regressions, fallback-policy scenarios, and TUI redaction/limit
tests so the doc matches shipped behavior.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
modules/cerebro/tests/storage_config_test.rs (1)
153-159:⚠️ Potential issue | 🟠 MajorDo not shadow the env lock guard before setting
CEREBRO_TEST_FAIL_STORAGE.Line 157 shadows the lock guard from Line 153, which drops
ENV_LOCKbefore Line 159 mutates the environment variable.Suggested fix
- let _guard = ENV_LOCK.lock().expect("env lock"); + let _env_lock = ENV_LOCK.lock().expect("env lock"); @@ - let _guard = tracing::subscriber::set_default(subscriber); + let _subscriber_guard = tracing::subscriber::set_default(subscriber);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/cerebro/tests/storage_config_test.rs` around lines 153 - 159, The test is shadowing and dropping the ENV_LOCK guard by reusing `_guard` before calling EnvVarGuard::set, which allows concurrent env mutation; rename the second `_guard` (from tracing::subscriber::set_default) to a different variable (e.g., `tracing_guard`) or drop the first only after EnvVarGuard::set so that ENV_LOCK remains held across the environment mutation; ensure ENV_LOCK.lock().expect("env lock") stays live while calling EnvVarGuard::set("CEREBRO_TEST_FAIL_STORAGE", "1").openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md (1)
46-47:⚠️ Potential issue | 🟠 MajorResolve conflicting coverage outcomes in the same report.
Line 46 says coverage passed, but Line 117 says coverage reporting was not performed. Keep one auditable outcome and make both lines consistent.
Suggested edit
-Migration and embedded-default behaviors verified with targeted tests, but coverage reporting was not performed in this run. +Migration and embedded-default behaviors verified with targeted tests, and coverage passed via `make test-coverage` (artifact: `gradle/aggregation/build/reports/kover/html/index.html`).Also applies to: 117-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md` around lines 46 - 47, The report contains conflicting coverage states: the "Coverage: ✅ Passed (threshold: 60%)" entry and a later "coverage reporting was not performed" entry; pick a single auditable outcome and make both lines consistent (either update the earlier "Coverage: ✅ Passed..." line to reflect that coverage was not performed, or remove/replace the later note and ensure the passed status includes threshold and report reference). Locate the two entries (the "Coverage: ✅ Passed (threshold: 60%)" line and the later "coverage reporting was not performed" note) and edit them so the document shows one clear, consistent coverage result and include any necessary context (threshold, report ID or reason for omission) for auditability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lychee.toml`:
- Around line 15-17: Update the two regex exclusions: for the pattern
'^https://docs\.github\.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets(?:[?].*)?$'
add an optional trailing slash before the query group (i.e. make the segment
before the query 'use-secrets/?') so URLs with a trailing slash are matched; for
the pattern '^https://packages\.ubuntu\.com/?(?:[?].*)?$' replace the root-only
matcher with a path-aware alternative (e.g. allow '(/.*)?$') so paths like
'/jammy/openssl' are also excluded; update the two regex strings accordingly in
the config.
In `@modules/cerebro/src/migration/legacy.rs`:
- Around line 69-85: The normalize_export function can produce duplicate
memory_id values after calling normalize_memory_id, so add a uniqueness check
while building memory records: track seen ids (e.g., a HashSet<String>) when
iterating export.memory in normalize_export, and if
normalize_memory_id(&record.id) produces an id already in the set, return a
MigrationError with a clear structured message including the conflicting source
id and memory_id instead of proceeding; only insert the MemoryRecord and add the
id to the set when it's unique, and keep the existing memory.sort_by(...)
afterwards. Also apply the same uniqueness-check pattern for the other similar
block referenced (lines ~115-117) to ensure no collisions are missed.
In `@modules/cerebro/src/migration/mod.rs`:
- Around line 98-107: The code currently ignores poisoned STORAGE_CACHE locks by
only handling Ok(cache) from cache.lock() and continuing silently on Err; update
both places that call STORAGE_CACHE.lock() (the initial cache lookup and the
later cache insertion block) to treat Err as a real failure and return an
internal error rather than proceeding: check the Result from cache.lock(), and
on Err (poisoned lock) return an appropriate internal error (e.g. convert the
poison error into your crate's internal error variant) before creating or using
SurrealStorage::new_embedded or attempting to read/insert by cache_key.
In `@modules/cerebro/src/storage/surreal.rs`:
- Around line 53-57: The public constructor RemoteSurreal::new_remote currently
returns CerebroError::NotImplemented while StorageMode::RemoteSurreal can still
be selected in configuration; either implement the remote SurrealDB client in
new_remote or prevent this mode at config-validation time. Update new_remote
(symbol: new_remote) to construct and return a working RemoteSurreal instance
using CerebroConfig (symbols: RemoteSurreal, CerebroConfig) or, if not
implementing now, add validation that rejects StorageMode::RemoteSurreal during
config parsing/validation (reference StorageMode::RemoteSurreal) and return a
clear CerebroError there so the app cannot start with an unsupported mode.
- Around line 394-412: In the count() async method update the SurrealDB query
string to the 3.x syntax by replacing "SELECT count() AS count FROM memory;"
with "SELECT count FROM memory GROUP ALL;" so the aggregate uses count without
parentheses and includes GROUP ALL; keep the existing response handling
(check(), take(0), first().get("count").and_then(Value::as_u64)) to extract the
aliased count and return it as usize, leaving the CerebroError mapping
unchanged.
In `@openspec/specs/cerebro/spec.md`:
- Around line 24-30: The architecture text incorrectly claims SurrealDB can be
"embedded or remote" while the code currently rejects
StorageMode::RemoteSurreal; update the spec wording in Cerebro's architecture
description to reflect shipped behavior by either removing "remote" and stating
"embedded only" or explicitly marking remote SurrealDB as
"future/feature‑gated/unavailable in this build" (reference
StorageMode::RemoteSurreal and the Cerebro MCP Server description) so the docs
match the implementation.
In `@README.md`:
- Around line 194-199: The "Cerebro storage & operations" section currently
conflicts with nearby text that says the SurrealDB backend was removed; update
the heading and surrounding copy to explicitly state the scope change (e.g.,
"SurrealDB removed as runtime memory backend; retained only as Cerebro embedded
storage/migration backend") so operators can't misconfigure systems, and ensure
phrases in this section (and any bullets referencing migration, CLI, TUI,
operational notes) consistently reflect that SurrealDB is no longer a general
runtime backend but only used for embedded Cerebro storage/migration.
- Around line 194-203: The new user-facing "Cerebro storage & operations"
section (and the "DeepWiki" badge area) lacks Spanish parity; either add the
Spanish translation for this section or insert an explicit note that Spanish
translation is pending (e.g., a short line like "ES: Traducción pendiente" under
the English heading). Update the README.md content around the "Cerebro storage &
operations" heading (and the new operations guidance bullet list) to include the
ES counterpart or the pending-translation note so user-facing docs maintain
EN/ES parity per guidelines.
---
Duplicate comments:
In `@modules/cerebro/tests/storage_config_test.rs`:
- Around line 153-159: The test is shadowing and dropping the ENV_LOCK guard by
reusing `_guard` before calling EnvVarGuard::set, which allows concurrent env
mutation; rename the second `_guard` (from tracing::subscriber::set_default) to
a different variable (e.g., `tracing_guard`) or drop the first only after
EnvVarGuard::set so that ENV_LOCK remains held across the environment mutation;
ensure ENV_LOCK.lock().expect("env lock") stays live while calling
EnvVarGuard::set("CEREBRO_TEST_FAIL_STORAGE", "1").
In
`@openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md`:
- Around line 46-47: The report contains conflicting coverage states: the
"Coverage: ✅ Passed (threshold: 60%)" entry and a later "coverage reporting was
not performed" entry; pick a single auditable outcome and make both lines
consistent (either update the earlier "Coverage: ✅ Passed..." line to reflect
that coverage was not performed, or remove/replace the later note and ensure the
passed status includes threshold and report reference). Locate the two entries
(the "Coverage: ✅ Passed (threshold: 60%)" line and the later "coverage
reporting was not performed" note) and edit them so the document shows one
clear, consistent coverage result and include any necessary context (threshold,
report ID or reason for omission) for auditability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4382a540-6d3f-4e2e-bc3d-100b535c49cd
📒 Files selected for processing (17)
README.mdlychee.tomlmodules/cerebro/src/migration/legacy.rsmodules/cerebro/src/migration/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/src/tui/views/mod.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/tests/tui_toggle_tests.rsopenspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdopenspec/specs/cerebro/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: sonar
- GitHub Check: report / Contributor Quality Report
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdREADME.mdopenspec/specs/cerebro/spec.md
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdmodules/cerebro/src/tui/views/mod.rslychee.tomlREADME.mdmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/tests/tui_shutdown_tests.rsopenspec/specs/cerebro/spec.mdmodules/cerebro/src/migration/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
modules/cerebro/src/tui/views/mod.rsmodules/cerebro/src/tools.rsmodules/cerebro/src/tui/redaction.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/storage_config_test.rsmodules/cerebro/src/migration/legacy.rsmodules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/storage/surreal.rsmodules/cerebro/tests/tui_shutdown_tests.rsmodules/cerebro/src/migration/mod.rs
🧠 Learnings (10)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/tests/migration_legacy_test.rsmodules/cerebro/tests/tui_shutdown_tests.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.mdmodules/cerebro/src/tools.rsmodules/cerebro/tests/tui_non_blocking_tests.rsmodules/cerebro/tests/cli_migration_test.rsmodules/cerebro/src/tui/mod.rsmodules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/tests/cli_migration_test.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/src/tui/redaction.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
modules/cerebro/src/tools.rsmodules/cerebro/tests/cli_migration_test.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
modules/cerebro/src/tui/redaction.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Inspect existing module and adjacent tests before editing; define scope boundary with one concern per PR and avoid mixed feature+refactor+infra patches
Applied to files:
modules/cerebro/tests/tui_toggle_tests.rsmodules/cerebro/tests/tui_shutdown_tests.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Keep each iteration reversible with small commits and clear rollback strategy; validate assumptions with code search before implementing
Applied to files:
modules/cerebro/src/storage/surreal.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
modules/cerebro/src/storage/surreal.rs
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-18-2026-03-17-cerebro-embedded-surrealdb-migration-phase1/verify-report.md
[warning] 87-87: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (17)
README.md (2)
201-203: Good heading semantics for DeepWiki section.Using a descriptive heading with the badge on the next line is clean and accessible.
196-199: All four anchor links in README lines 196–199 are correct and resolve to existing headings in the target migration.md file:
#cerebro-storage-defaults-embedded→ "Cerebro storage defaults (embedded)" (line 94)#migration-cli→ "Migration CLI" (line 143)#optional-tui-operator-only→ "Optional TUI (operator-only)" (line 110)#operational-notes→ "Operational notes" (line 164)No updates needed.
modules/cerebro/src/tui/views/mod.rs (1)
32-55: Tabbed header layout looks correct now.Separate
tabs_area/meta_arearendering withConstraint::Length(3)avoids overlap and preserves tab visibility.modules/cerebro/src/tools.rs (1)
40-47: Redaction/extraction alignment is in good shape.
mem_suggest_topic_keyno longer leaks rawseed, andmem_timelinenow has matching safe output extraction foritems_count.Also applies to: 276-281, 335-346
modules/cerebro/src/tui/redaction.rs (1)
87-109: Recursive allowlist handling is correctly applied.Passing the active allowlist through nested objects/arrays keeps redaction behavior consistent and secure-by-default.
modules/cerebro/tests/cli_migration_test.rs (1)
1-82: LGTM!Past review concerns are addressed:
- Path handling now uses
.arg(source.as_path())instead of.to_str().unwrap()(lines 16, 18, 31, 33, 54, 56, 76, 78)- Mismatch mutation is now explicitly asserted with
.expect("memory array")andassert!(!memory.is_empty(), ...)(lines 63-68)Test coverage for CLI migration workflow is solid.
modules/cerebro/tests/migration_legacy_test.rs (1)
1-26: LGTM!Past review concern addressed: fixture paths now use
env!("CARGO_MANIFEST_DIR")for deterministic loading (lines 6-7, 17-18).modules/cerebro/src/storage/surreal.rs (2)
65-132: Transaction-based batch writes address atomicity concern.Good implementation using
BEGIN/COMMITto ensure all-or-nothing semantics for migration imports.
341-392: Server-side filtering addresses past performance concern.Search now uses parameterized SurrealQL with
WHEREclauses andLIMITinstead of client-side filtering.modules/cerebro/src/tui/mod.rs (5)
42-54: Error variants are now semantically correct.Good addition of
EventFailedandRenderFailedto distinguish runtime failures from initialization errors.
314-332: Event buffer now uses config with minimum guard.
event_buffer.max(1)prevents zero-capacity buffer issues.
364-377: Deduplicated storage queries.Single query result now populates both
memory_itemsandtimeline_items, addressing the duplicate query concern.
250-255: TerminalGuard Drop ensures cleanup on panic.Good defensive pattern—terminal state is restored even if the TUI task panics.
92-108: Panic isolation via catch_unwind.TUI panics are caught and logged without crashing the host process. The spawned task properly handles both panic and error paths.
modules/cerebro/tests/tui_toggle_tests.rs (1)
1-95: LGTM!All past review concerns addressed:
ENV_LOCKacquired in async test (line 56)EnvVarGuardensures env restoration on panic (lines 12-39)tx.send(true).expect(...)asserts success (line 72)timeout(Duration::from_secs(1), ...)prevents hangs (lines 73-76)modules/cerebro/tests/tui_shutdown_tests.rs (1)
1-83: LGTM!All past review concerns addressed:
EnvVarGuardRAII pattern for env cleanup (lines 12-33)- Timeouts on
handle.join()prevent hangs (lines 54-57, 79-82)- Crash isolation test correctly expects
join()to succeed since panics are caught instart_tui_task.modules/cerebro/tests/tui_non_blocking_tests.rs (1)
1-88: LGTM!All past review concerns addressed:
- Timeout relaxed from 50ms to 500ms (lines 46, 77)
EnvVarGuardRAII pattern for env cleanup (lines 12-35)ENV_LOCKserializes env-mutating tests
| pub fn new_remote(_config: &CerebroConfig) -> Result<Self, CerebroError> { | ||
| Err(CerebroError::NotImplemented( | ||
| "remote surrealdb storage is not available in this build".to_string(), | ||
| )) | ||
| } |
There was a problem hiding this comment.
RemoteSurreal remains unimplemented but publicly configurable.
This stub returns NotImplemented while StorageMode::RemoteSurreal is selectable in config. Consider either implementing the remote client or rejecting this mode during config validation to avoid runtime surprises.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/cerebro/src/storage/surreal.rs` around lines 53 - 57, The public
constructor RemoteSurreal::new_remote currently returns
CerebroError::NotImplemented while StorageMode::RemoteSurreal can still be
selected in configuration; either implement the remote SurrealDB client in
new_remote or prevent this mode at config-validation time. Update new_remote
(symbol: new_remote) to construct and return a working RemoteSurreal instance
using CerebroConfig (symbols: RemoteSurreal, CerebroConfig) or, if not
implementing now, add validation that rejects StorageMode::RemoteSurreal during
config parsing/validation (reference StorageMode::RemoteSurreal) and return a
clear CerebroError there so the app cannot start with an unsupported mode.
…ror handling in legacy migration
|
* feat(cerebro): add embedded migration and TUI Provide embedded SurrealDB migration tooling and an optional in-process TUI for observability without blocking MCP. * chore(deps): update dependencies in Cargo.lock * chore: ignore local cerebro db * fix(cerebro): enforce embedded config validation Load config files, validate embedded SurrealDB loopback/auth, report fallback mode, and expand migration tests. * fix(cerebro): normalize SurrealDB exports for 2.x migration * chore: apply spotless formatting to README * fix(cerebro): upgrade surrealdb to 3.x * fix(cerebro): update ratatui and lru * fix(cerebro): enhance migration tests and improve error handling in TUI * fix(cerebro): simplify response handling and improve count query logic * feat(cerebro): enhance Dockerfile for module structure and improve error handling in legacy migration
1 participant
Provide embedded SurrealDB migration tooling and an optional in-process TUI for observability without blocking MCP.
This pull request introduces significant enhancements to the Cerebro memory service, focusing on migration support, configuration flexibility, and operational tooling. The main changes include the addition of a migration subsystem for legacy data, expanded configuration options (including storage backends and a new terminal UI), and improved documentation for deployment and migration processes.
Migration and Storage Enhancements:
StorageModeandStorageFallbackenums, aSurrealConfigstruct for detailed SurrealDB settings, and validation logic to ensure safe use of remote SurrealDB backends (loopback and credential checks). [1] [2] [3]Operational and UI Improvements:
Documentation and Build System:
Cargo.tomlto add new dependencies (clap,crossterm,ratatui,sha2, etc.), define feature flags for TUI support, and clarify binary targets.Code Organization:
lib.rsto expose new configuration, migration, and TUI modules.These changes collectively modernize Cerebro's deployment, migration, and operational experience, making it safer and more flexible for both new and existing users.
Migration and Storage:
StorageMode,StorageFallback),SurrealConfig, and validation logic for remote SurrealDB. [1] [2] [3]Operational and UI:
Documentation and Build:
Code Organization:
lib.rsfor broader API access.