Conversation
- Memory v2 (categories, dream cleanup, self-skeptical) - System prompt hardened (30/30 tools, security, negative rules) - Security CI (audit, deny, dependency review) - Browser, debug mode, GitHub integration, profiles, pinning - tree-sitter, arboard, auto-skill, FTS search - UX: spinner, bell, help categories, quit confirm, Ctrl+R - 274 tests, 30 tools, 51 PRs
428b477 to
bccca14
Compare
📝 WalkthroughWalkthroughRelease preparation for v1.7.0 with version bump in Cargo.toml, expanded CHANGELOG with detailed release notes, updated README tool count (29→30), and four new documentation pages covering getting started, memory system architecture, configuration reference, and tools reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the project to version 1.7.0, introducing significant features such as a new memory system, browser automation, and enhanced security CI. The changelog and documentation have been updated to reflect these changes. Feedback identifies a discrepancy between the described and actual behavior of the /search-all command and suggests increasing test coverage to match the project's stated focus on quality and security.
| - **Agent profiles**: `/profile save/load/list` | ||
| - **Context pinning**: `/pin` — pinned messages survive compaction | ||
| - **Auto-skill creation**: auto-generate skills after complex tasks | ||
| - **Cross-session FTS search**: `/search-all` across all saved sessions |
There was a problem hiding this comment.
The changelog states that /search-all searches across "all saved sessions," but the implementation in mc-core/src/fts.rs (line 41) limits the search to the 100 most recent sessions and stops after finding 20 results (line 93). This discrepancy should be clarified in the documentation or the implementation should be updated to match the description.
| - **arboard** cross-platform clipboard (replaces shell pbcopy/xclip) | ||
| - **GitHub templates**: bug report, feature request, PR template | ||
| - **SECURITY.md**: vulnerability reporting policy | ||
| - **274 tests**, 50% coverage, 0 warnings |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/docs/guides/getting-started.md`:
- Line 66: The relative link "reference/config.md" in the getting-started
document is incorrect; update the link target used in the line 'See
[Configuration Reference](reference/config.md) for all options.' to use the
correct relative path "../reference/config.md" so the Configuration Reference
resolves from mc/docs/guides/getting-started.md.
In `@mc/docs/guides/memory.md`:
- Around line 17-23: The fenced code blocks showing CLI examples and directory
trees (e.g., the block with "/memory", "/memory get test_cmd", "/memory set
test_cmd \"pytest\"", and "/memory delete old_key" and the other blocks at lines
referenced) are missing language identifiers; update each fenced block to
include an appropriate language tag (use "text" or "bash" for shell/command
examples and "text" for the directory tree) so lint stops flagging them—apply
this to the blocks around the shown commands and the other blocks mentioned
(lines 51–56, 59–66, 72–78, 84–88).
In `@mc/docs/reference/config.md`:
- Around line 54-68: The fenced code block that lists CLI flags (the
triple-backtick block starting at the options list) lacks a language identifier
and triggers markdown-lint warnings; update that block to use a language tag
(e.g., add "text" after the opening triple backticks) so the block becomes
```text ... ``` to silence the lint and preserve plain-text formatting.
- Around line 11-30: The [default] example documents unsupported keys and
misplaces fields: remove or relocate the entries base_url, fallback_provider,
fallback_model, notifications, notification_webhook, compaction_threshold, and
compaction_preserve_recent from the [default] block and place them in their
correct sections or a “Runtime defaults” section; update the [default] snippet
to only show keys actually loaded from [default] (e.g., model, max_tokens,
provider, permission_mode) and add a short note that fallback provider/model and
notifications are runtime defaults and only applied when the current config
schema explicitly supports them.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2a8c7ad-84a1-4602-b707-cb43c4147f27
⛔ Files ignored due to path filters (1)
mc/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mdmc/Cargo.tomlmc/docs/guides/getting-started.mdmc/docs/guides/memory.mdmc/docs/reference/config.mdmc/docs/reference/tools.md
| notifications = true | ||
| ``` | ||
|
|
||
| See [Configuration Reference](reference/config.md) for all options. |
There was a problem hiding this comment.
Fix the relative link path to the config reference.
Line 66 currently links to reference/config.md, which is incorrect from mc/docs/guides/. Use ../reference/config.md to avoid a broken docs link.
Proposed fix
-See [Configuration Reference](reference/config.md) for all options.
+See [Configuration Reference](../reference/config.md) for all options.📝 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.
| See [Configuration Reference](reference/config.md) for all options. | |
| See [Configuration Reference](../reference/config.md) for all options. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/docs/guides/getting-started.md` at line 66, The relative link
"reference/config.md" in the getting-started document is incorrect; update the
link target used in the line 'See [Configuration Reference](reference/config.md)
for all options.' to use the correct relative path "../reference/config.md" so
the Configuration Reference resolves from mc/docs/guides/getting-started.md.
| ``` | ||
| /memory # list all facts | ||
| /memory get test_cmd # get specific fact | ||
| /memory set test_cmd "pytest" # save fact | ||
| /memory delete old_key # remove fact | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add language identifiers to plain fenced code blocks.
Several fenced blocks (Lines 17, 51, 59, 72, 84) omit language tags, which will keep triggering lint warnings.
Suggested update pattern
-```
+```text
/memory # list all facts
...
Apply similarly to the directory tree and command-list blocks.
</details>
Also applies to: 51-56, 59-66, 72-78, 84-88
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @mc/docs/guides/memory.md around lines 17 - 23, The fenced code blocks
showing CLI examples and directory trees (e.g., the block with "/memory",
"/memory get test_cmd", "/memory set test_cmd "pytest"", and "/memory delete
old_key" and the other blocks at lines referenced) are missing language
identifiers; update each fenced block to include an appropriate language tag
(use "text" or "bash" for shell/command examples and "text" for the directory
tree) so lint stops flagging them—apply this to the blocks around the shown
commands and the other blocks mentioned (lines 51–56, 59–66, 72–78, 84–88).
</details>
<!-- fingerprinting:phantom:triton:puma:06ee9ad6-cb94-4b18-bd94-909a13511a94 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| [default] | ||
| # LLM | ||
| model = "claude-sonnet-4-20250514" | ||
| max_tokens = 8192 | ||
| provider = "anthropic" | ||
| base_url = "" # custom API endpoint | ||
| fallback_provider = "" # secondary provider | ||
| fallback_model = "" # secondary model | ||
|
|
||
| # Permissions | ||
| permission_mode = "auto" # auto | allow | deny | prompt | ||
|
|
||
| # Context | ||
| compaction_threshold = 0.8 # compact at 80% context usage | ||
| compaction_preserve_recent = 4 # keep last 4 messages | ||
|
|
||
| # Notifications | ||
| notifications = true # bell + desktop notifications | ||
| notification_webhook = "" # Slack/Discord webhook URL | ||
|
|
There was a problem hiding this comment.
[default] section documents unsupported keys and wrong section placement.
Lines 16–30 include fields that are not loaded from [default] (base_url, fallback_provider, fallback_model, notifications, notification_webhook, compaction_*). This will mislead users because these settings won’t take effect as documented.
Proposed docs correction
[default]
# LLM
model = "claude-sonnet-4-20250514"
max_tokens = 8192
provider = "anthropic"
-base_url = "" # custom API endpoint
-fallback_provider = "" # secondary provider
-fallback_model = "" # secondary model
# Permissions
permission_mode = "auto" # auto | allow | deny | prompt
-
-# Context
-compaction_threshold = 0.8 # compact at 80% context usage
-compaction_preserve_recent = 4 # keep last 4 messages
-
-# Notifications
-notifications = true # bell + desktop notifications
-notification_webhook = "" # Slack/Discord webhook URL
+
+[compaction]
+auto_compact_threshold = 0.8
+preserve_recent_messages = 4Also add a short note that fallback provider/model and notifications are runtime defaults unless explicitly supported by current config schema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/docs/reference/config.md` around lines 11 - 30, The [default] example
documents unsupported keys and misplaces fields: remove or relocate the entries
base_url, fallback_provider, fallback_model, notifications,
notification_webhook, compaction_threshold, and compaction_preserve_recent from
the [default] block and place them in their correct sections or a “Runtime
defaults” section; update the [default] snippet to only show keys actually
loaded from [default] (e.g., model, max_tokens, provider, permission_mode) and
add a short note that fallback provider/model and notifications are runtime
defaults and only applied when the current config schema explicitly supports
them.
| ``` | ||
| --model <MODEL> LLM model | ||
| --provider <PROVIDER> Provider name | ||
| --max-tokens <N> Max tokens per response | ||
| --resume Resume last session | ||
| --session-id <ID> Resume specific session | ||
| --pipe Read from stdin | ||
| --json JSON output mode | ||
| --yes Auto-approve (CI/CD) | ||
| --trace Debug logging | ||
| --validate-config Validate and exit | ||
| --max-budget-usd <N> Cost limit | ||
| --max-turns <N> Turn limit | ||
| --add-dir <DIR> Grant access to extra directories | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced CLI block.
Line 54 starts a fenced code block without language, which triggers markdown lint warnings.
Proposed fix
-```
+```text
--model <MODEL> LLM model
--provider <PROVIDER> Provider name
...
--add-dir <DIR> Grant access to extra directories</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/docs/reference/config.md` around lines 54 - 68, The fenced code block that
lists CLI flags (the triple-backtick block starting at the options list) lacks a
language identifier and triggers markdown-lint warnings; update that block to
use a language tag (e.g., add "text" after the opening triple backticks) so the
block becomes ```text ... ``` to silence the lint and preserve plain-text
formatting.
Version bump + CHANGELOG for v1.7.0. See CHANGELOG.md for full details.
Summary by CodeRabbit
Release v1.7.0
New Features
Documentation