feat: auto-skill creation + cross-session FTS search (inspired by Hermes)#48
Merged
kienbui1995 merged 2 commits intomainfrom Apr 13, 2026
Merged
feat: auto-skill creation + cross-session FTS search (inspired by Hermes)#48kienbui1995 merged 2 commits intomainfrom
kienbui1995 merged 2 commits intomainfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdded cross-session full-text search and an auto-skill generator. New public modules Changes
Sequence Diagram(s)sequenceDiagram
participant TUI as TUI (mc-tui)
participant CLI as CLI (mc-cli)
participant FTS as FTS (mc_core::fts)
participant FS as Filesystem
TUI->>CLI: enqueue SearchAll(query)
CLI->>FTS: search_all_sessions(sessions_dir, query)
FTS->>FS: read sessions_dir/*.json (sorted by mtime)
FS-->>FTS: file contents
FTS->>FTS: deserialize session, extract blocks, match query, build snippets
FTS-->>CLI: Vec<FtsResult>
CLI->>TUI: render results (header / lines / snippets)
sequenceDiagram
participant Runtime as ConversationRuntime
participant Auto as AutoSkill (mc_core::auto_skill)
participant FS as Filesystem
participant Provider as ProviderEvent
Runtime->>Runtime: finalize turn, collect tool_calls & final_text
Runtime->>Auto: should_create_skill(count, had_errors)
alt should_create == true and HOME set
Runtime->>Auto: generate_skill_content(summary, tools_used)
Auto->>FS: create skills_dir/auto, create_new safe filename, write .md
FS-->>Auto: write result (path or error)
Auto-->>Runtime: saved path (Some) / None
Runtime->>Provider: emit ProviderEvent::TextDelta("Saved auto skill: <path>")
end
Runtime-->>Caller: return TurnResult (unchanged)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
mc/crates/mc-core/src/auto_skill.rs (1)
35-37: Consider returningResultinstead of collapsing all I/O errors toNone.Current behavior loses failure reason, which makes runtime diagnostics and user-facing feedback harder.
Refactor sketch
-pub fn save_auto_skill(skills_dir: &Path, name: &str, content: &str) -> Option<String> { +pub fn save_auto_skill(skills_dir: &Path, name: &str, content: &str) -> std::io::Result<Option<String>> { @@ - if std::fs::create_dir_all(&dir).is_err() { - return None; - } + std::fs::create_dir_all(&dir)?; @@ - return None; // don't overwrite existing skills + return Ok(None); // don't overwrite existing skills } - std::fs::write(&path, content).ok()?; - Some(path.to_string_lossy().to_string()) + std::fs::write(&path, content)?; + Ok(Some(path.to_string_lossy().to_string())) }Also applies to: 52-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/auto_skill.rs` around lines 35 - 37, The code collapses all I/O failures into None by calling std::fs::create_dir_all(&dir).is_err() and returning None; change the enclosing function(s) in mc::auto_skill.rs to return Result<T, std::io::Error> (or a crate error type) instead of Option, replace the is_err() checks with match or ? to propagate the std::io::Error from std::fs::create_dir_all(&dir), and update the two spots (the create_dir_all call around lines 35–37 and the similar call at 52–53) to return Err(e) (or propagate with ?) so callers can handle/inspect the real I/O error; remember to update callers' signatures and call sites accordingly.
🤖 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/crates/mc-core/src/auto_skill.rs`:
- Line 33: The public function save_auto_skill returns an Option<String> and its
result must not be dropped silently; add the #[must_use] attribute directly
above the pub fn save_auto_skill(...) declaration so callers get a compiler
warning if they ignore the return value (apply the same pattern to other public
functions returning values where needed).
- Around line 77-83: The test creates a temp directory name using only
std::process::id(), which can collide when tests run in parallel; change the
test to append a unique atomic counter to the temp path (e.g., use a static
AtomicUsize or AtomicU64 incremented with fetch_add) so the dir becomes
std::env::temp_dir().join(format!("mc-skill-{}-{}", std::process::id(),
UNIQUE_COUNTER.fetch_add(1, Ordering::SeqCst))); update the test that calls
save_auto_skill (the variables dir, path, path2) to use this new unique dir and
keep the same assertions and cleanup.
- Around line 49-53: Replace the TOCTOU pattern that checks path.exists() then
calls std::fs::write by atomically creating the file with
OpenOptions::new().write(true).create_new(true).open(...) and writing content
via write_all so creation fails if the file already exists (reference the
variables path and content and the current std::fs::write call). In tests that
currently use process::id() for temp directory names, switch to a global atomic
counter (e.g., an AtomicUsize in the test module) to generate unique suffixes
per test run to avoid collisions when tests run in parallel.
In `@mc/crates/mc-core/src/fts.rs`:
- Around line 57-71: FTS currently only searches crate::session::Block::Text and
skips other text-bearing variants; update the inner loop over session.messages
in mc/crates/mc-core/src/fts.rs to also extract and search text from
Block::Thinking { text }, Block::ToolUse { input }, and Block::ToolResult {
output } (use a match that binds a &str for each variant), then reuse the
existing search/slicing logic (q, start/end, snippet, file_name,
session.created_at, results.push) so command inputs, tool outputs and thinking
text are included in results.
- Around line 60-66: The code computes pos from text.to_lowercase().find(&q) but
then uses that byte offset to slice the original text, which can panic on
Unicode boundaries; update the logic in the FTS search where pos, start, end,
and snippet are computed (references: text, q, pos, start, end, snippet,
FtsResult) to derive safe byte indices from character indices—e.g., iterate
text.char_indices() to map the lowercase match position back to the original
string’s byte offsets (or perform the find on a normalized buffer and slice that
same buffer) so slicing uses valid UTF-8 byte boundaries and cannot panic.
In `@mc/crates/mc-core/src/runtime.rs`:
- Around line 683-686: The filename generation for auto skills uses
format!("auto-{iterations}t-{}", tool_calls.len()) inside the call to
crate::auto_skill::save_auto_skill which causes collisions; change the naming to
include a unique suffix (e.g., current timestamp, a monotonic counter, or a
short hash of content) so each save_auto_skill call produces a distinct name
(for example add chrono::Utc::now().timestamp_millis(), a global atomic counter,
or a content hash into the formatted string) and pass that new unique filename
to crate::auto_skill::save_auto_skill to preserve non-overwrite behavior while
still keeping the iterations/tool count in the name.
- Around line 676-677: The current had_errors value is computed from tool_calls
(a Vec of tool names) so t.contains("error") checks names, not execution
results; update the code that pushes into tool_calls (the two push sites) to
also record the failure flag (either change tool_calls to store a struct/tuple
like (name, is_error) or maintain a parallel Vec<bool> tool_errors), compute
had_errors from those recorded is_error values, and then call
crate::auto_skill::should_create_skill(tool_calls.len(), had_errors) using the
real failure-derived had_errors; ensure any code reading tool_calls is adjusted
to the new shape or continues to read only names.
In `@mc/crates/mc-core/src/session.rs`:
- Around line 191-193: The Session struct's created_at field remains empty
because Session::default() (used by Session::default in runtime and propagated
by branch code) leaves created_at as "" due to #[serde(default)]; set created_at
to the current timestamp when a new Session is constructed or immediately before
the first save: update the Session::default() (or the factory that creates
sessions) to assign created_at = Utc::now().to_rfc3339() (or equivalent
timestamp provider) so newly created sessions serialize a real timestamp and
downstream code (e.g., search-all) can rely on it.
In `@mc/crates/mc-tui/src/commands.rs`:
- Line 49: The /search-all branch is wired but not discoverable; add the
"/search-all" entry to the application's slash-command registry
(App::SLASH_COMMANDS) and include it in the help/discoverability list so it
appears in /help and Tab completion; ensure the help text/usage string matches
the one used in commands.rs (the Usage: /search-all <keyword> …) and that the
command maps to PendingCommand::SearchAll to keep behavior consistent.
---
Nitpick comments:
In `@mc/crates/mc-core/src/auto_skill.rs`:
- Around line 35-37: The code collapses all I/O failures into None by calling
std::fs::create_dir_all(&dir).is_err() and returning None; change the enclosing
function(s) in mc::auto_skill.rs to return Result<T, std::io::Error> (or a crate
error type) instead of Option, replace the is_err() checks with match or ? to
propagate the std::io::Error from std::fs::create_dir_all(&dir), and update the
two spots (the create_dir_all call around lines 35–37 and the similar call at
52–53) to return Err(e) (or propagate with ?) so callers can handle/inspect the
real I/O error; remember to update callers' signatures and call sites
accordingly.
🪄 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: 8fd65e52-05ba-462b-bb9e-2f016d0a0b02
📒 Files selected for processing (9)
mc/crates/mc-cli/src/main.rsmc/crates/mc-core/src/auto_skill.rsmc/crates/mc-core/src/branch.rsmc/crates/mc-core/src/fts.rsmc/crates/mc-core/src/lib.rsmc/crates/mc-core/src/runtime.rsmc/crates/mc-core/src/session.rsmc/crates/mc-tui/src/app.rsmc/crates/mc-tui/src/commands.rs
Comment on lines
+77
to
+83
| let dir = std::env::temp_dir().join(format!("mc-skill-{}", std::process::id())); | ||
| let path = save_auto_skill(&dir, "test-skill", "# Test"); | ||
| assert!(path.is_some()); | ||
| // Don't overwrite | ||
| let path2 = save_auto_skill(&dir, "test-skill", "# Test 2"); | ||
| assert!(path2.is_none()); | ||
| std::fs::remove_dir_all(&dir).ok(); |
There was a problem hiding this comment.
Use an atomic counter in test temp paths to avoid parallel collisions.
Line 77 uses only process ID, which can collide across tests in the same process.
Proposed fix
#[cfg(test)]
mod tests {
use super::*;
+ use std::sync::atomic::{AtomicUsize, Ordering};
+
+ static TEMP_COUNTER: AtomicUsize = AtomicUsize::new(0);
@@
fn save_skill() {
- let dir = std::env::temp_dir().join(format!("mc-skill-{}", std::process::id()));
+ let unique = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed);
+ let dir = std::env::temp_dir()
+ .join(format!("mc-skill-{}-{unique}", std::process::id()));
let path = save_auto_skill(&dir, "test-skill", "# Test");
assert!(path.is_some());As per coding guidelines: "Use atomic counter for unique temp file paths in tests to avoid parallel test collisions".
📝 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.
Suggested change
| let dir = std::env::temp_dir().join(format!("mc-skill-{}", std::process::id())); | |
| let path = save_auto_skill(&dir, "test-skill", "# Test"); | |
| assert!(path.is_some()); | |
| // Don't overwrite | |
| let path2 = save_auto_skill(&dir, "test-skill", "# Test 2"); | |
| assert!(path2.is_none()); | |
| std::fs::remove_dir_all(&dir).ok(); | |
| #[cfg(test)] | |
| mod tests { | |
| use super::*; | |
| use std::sync::atomic::{AtomicUsize, Ordering}; | |
| static TEMP_COUNTER: AtomicUsize = AtomicUsize::new(0); | |
| #[test] | |
| fn save_skill() { | |
| let unique = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed); | |
| let dir = std::env::temp_dir() | |
| .join(format!("mc-skill-{}-{unique}", std::process::id())); | |
| let path = save_auto_skill(&dir, "test-skill", "# Test"); | |
| assert!(path.is_some()); | |
| // Don't overwrite | |
| let path2 = save_auto_skill(&dir, "test-skill", "# Test 2"); | |
| assert!(path2.is_none()); | |
| std::fs::remove_dir_all(&dir).ok(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/auto_skill.rs` around lines 77 - 83, The test creates a
temp directory name using only std::process::id(), which can collide when tests
run in parallel; change the test to append a unique atomic counter to the temp
path (e.g., use a static AtomicUsize or AtomicU64 incremented with fetch_add) so
the dir becomes std::env::temp_dir().join(format!("mc-skill-{}-{}",
std::process::id(), UNIQUE_COUNTER.fetch_add(1, Ordering::SeqCst))); update the
test that calls save_auto_skill (the variables dir, path, path2) to use this new
unique dir and keep the same assertions and cleanup.
mc/crates/mc-core/src/fts.rs
Outdated
Comment on lines
+60
to
+66
| if let Some(pos) = text.to_lowercase().find(&q) { | ||
| let start = pos.saturating_sub(60); | ||
| let end = (pos + q.len() + 60).min(text.len()); | ||
| results.push(FtsResult { | ||
| session_file: file_name.clone(), | ||
| timestamp: session.created_at.clone(), | ||
| snippet: format!("...{}...", &text[start..end]), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "fts.rs" | grep mc-coreRepository: kienbui1995/mc-code
Length of output: 93
🏁 Script executed:
cat -n mc/crates/mc-core/src/fts.rs | head -100Repository: kienbui1995/mc-code
Length of output: 3550
The snippet slice can panic on valid Unicode content.
pos is computed from text.to_lowercase().find(&q), then reused to slice the original text. When lowercasing changes byte lengths of characters (e.g., 'ẞ' → 'ß'), the byte offset becomes misaligned between the lowercase and original versions. Using misaligned offsets to slice the original text can panic if they land on invalid UTF-8 boundaries. Compute snippet bounds from text.char_indices() or keep search and slicing on the same normalized buffer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/fts.rs` around lines 60 - 66, The code computes pos
from text.to_lowercase().find(&q) but then uses that byte offset to slice the
original text, which can panic on Unicode boundaries; update the logic in the
FTS search where pos, start, end, and snippet are computed (references: text, q,
pos, start, end, snippet, FtsResult) to derive safe byte indices from character
indices—e.g., iterate text.char_indices() to map the lowercase match position
back to the original string’s byte offsets (or perform the find on a normalized
buffer and slice that same buffer) so slicing uses valid UTF-8 byte boundaries
and cannot panic.
Comment on lines
+676
to
+677
| let had_errors = tool_calls.iter().any(|t| t.contains("error")); | ||
| if crate::auto_skill::should_create_skill(tool_calls.len(), had_errors) { |
There was a problem hiding this comment.
had_errors is derived from tool names, not tool failures.
tool_calls only contains the names pushed at mc/crates/mc-core/src/runtime.rs:479 and mc/crates/mc-core/src/runtime.rs:495, so t.contains("error") is almost always false. Failed tool executions will still pass the “no errors” gate and produce auto-skills unless you track the actual is_error values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/runtime.rs` around lines 676 - 677, The current
had_errors value is computed from tool_calls (a Vec of tool names) so
t.contains("error") checks names, not execution results; update the code that
pushes into tool_calls (the two push sites) to also record the failure flag
(either change tool_calls to store a struct/tuple like (name, is_error) or
maintain a parallel Vec<bool> tool_errors), compute had_errors from those
recorded is_error values, and then call
crate::auto_skill::should_create_skill(tool_calls.len(), had_errors) using the
real failure-derived had_errors; ensure any code reading tool_calls is adjusted
to the new shape or continues to read only names.
Comment on lines
191
to
+193
| #[serde(default)] | ||
| pub created_at: String, | ||
| #[serde(default)] |
There was a problem hiding this comment.
created_at is never populated for new sessions.
#[serde(default)] keeps old JSON loadable, but new sessions still start as Session::default() in mc/crates/mc-core/src/runtime.rs:103, and mc/crates/mc-core/src/branch.rs:41-48 only propagates that value. As written, newly saved sessions serialize created_at: "", so /search-all cannot return the timestamp promised by this feature unless you stamp it when the session is created or before first save.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/session.rs` around lines 191 - 193, The Session
struct's created_at field remains empty because Session::default() (used by
Session::default in runtime and propagated by branch code) leaves created_at as
"" due to #[serde(default)]; set created_at to the current timestamp when a new
Session is constructed or immediately before the first save: update the
Session::default() (or the factory that creates sessions) to assign created_at =
Utc::now().to_rfc3339() (or equivalent timestamp provider) so newly created
sessions serialize a real timestamp and downstream code (e.g., search-all) can
rely on it.
| "/init" => app.pending_command = Some(PendingCommand::Init), | ||
| "/summary" => app.pending_command = Some(PendingCommand::Summary), | ||
| "/search" => if arg.is_empty() { app.push("Usage: /search <keyword>"); } else { app.pending_command = Some(PendingCommand::Search(arg.into())); }, | ||
| "/search-all" => if arg.is_empty() { app.push("Usage: /search-all <keyword> — search across all saved sessions"); } else { app.pending_command = Some(PendingCommand::SearchAll(arg.into())); }, |
There was a problem hiding this comment.
Keep /search-all in the discoverability lists.
This branch wires the command correctly, but "/search-all" is still missing from the /help output here and from App::SLASH_COMMANDS in mc/crates/mc-tui/src/app.rs:349-427, so users cannot discover it via help or Tab completion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-tui/src/commands.rs` at line 49, The /search-all branch is wired
but not discoverable; add the "/search-all" entry to the application's
slash-command registry (App::SLASH_COMMANDS) and include it in the
help/discoverability list so it appears in /help and Tab completion; ensure the
help text/usage string matches the one used in commands.rs (the Usage:
/search-all <keyword> …) and that the command maps to PendingCommand::SearchAll
to keep behavior consistent.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
mc/crates/mc-core/src/runtime.rs (1)
676-677:⚠️ Potential issue | 🟠 Major
had_errorsstill checks tool names, not execution results.The
tool_callsvector only contains tool names (populated at lines 479 and 495 with.name.clone()). Theis_errorflag from tool execution is available at both sites but never captured:
- Line 479:
r.is_erroris present in batch results- Line 495:
is_erroris returned fromexecute_toolTo fix, either change
tool_callstoVec<(String, bool)>storing(name, is_error), or maintain a separateVec<bool>for error flags.,
🐛 Proposed fix to track actual tool errors
- let mut tool_calls = Vec::new(); + let mut tool_calls: Vec<(String, bool)> = Vec::new();At line 479:
- tool_calls.push(r.name.clone()); + tool_calls.push((r.name.clone(), r.is_error));At line 495:
- tool_calls.push(name.clone()); + tool_calls.push((name.clone(), is_error));Then at line 676:
- let had_errors = tool_calls.iter().any(|t| t.contains("error")); - if crate::auto_skill::should_create_skill(tool_calls.len(), had_errors) { + let had_errors = tool_calls.iter().any(|(_, is_error)| *is_error); + if crate::auto_skill::should_create_skill(tool_calls.len(), had_errors) {Note: This also requires updating
TurnResult.tool_callsand extracting just names where needed (e.g., line 682 forgenerate_skill_content).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/runtime.rs` around lines 676 - 677, The code currently records only tool names in tool_calls so had_errors = tool_calls.iter().any(|t| t.contains("error")) inspects names instead of execution results; update the flow to propagate execution error flags from r.is_error (batch results) and the is_error returned by execute_tool into the recorded data (either change tool_calls to Vec<(String, bool)> or keep a parallel Vec<bool>), update TurnResult.tool_calls accordingly, capture and store the boolean error when pushing at the sites that now push .name.clone() (the batch handling around r.is_error and the execute_tool call), and then change the had_errors check used by crate::auto_skill::should_create_skill and any subsequent usages (e.g., generate_skill_content) to inspect the stored booleans or extract names-only where needed.
🧹 Nitpick comments (1)
mc/crates/mc-core/src/runtime.rs (1)
678-679:HOMEenvironment variable is Unix-specific.
std::env::var_os("HOME")returnsNoneon Windows, so auto-skill creation will silently be skipped on that platform. If cross-platform support is intended, consider using thedirscrate (dirs::config_dir()) orstd::env::var_os("USERPROFILE")as a Windows fallback.♻️ Cross-platform alternative
- let skills_dir = std::env::var_os("HOME") - .map(|h| std::path::PathBuf::from(h).join(".config/magic-code/skills")); + let skills_dir = dirs::config_dir() + .map(|c| c.join("magic-code/skills"));This would require adding
dirsto dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/runtime.rs` around lines 678 - 679, The code computing skills_dir via std::env::var_os("HOME") is Unix-specific and will be None on Windows; update the logic that sets skills_dir in runtime.rs (the variable named skills_dir) to use a cross-platform config directory (e.g., use dirs::config_dir() and join ".magic-code/skills") or at minimum fall back to std::env::var_os("USERPROFILE") when HOME is missing; if you choose dirs::config_dir(), add the dirs crate to Cargo.toml and ensure you handle Option results and join the path similarly to the original code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@mc/crates/mc-core/src/runtime.rs`:
- Around line 676-677: The code currently records only tool names in tool_calls
so had_errors = tool_calls.iter().any(|t| t.contains("error")) inspects names
instead of execution results; update the flow to propagate execution error flags
from r.is_error (batch results) and the is_error returned by execute_tool into
the recorded data (either change tool_calls to Vec<(String, bool)> or keep a
parallel Vec<bool>), update TurnResult.tool_calls accordingly, capture and store
the boolean error when pushing at the sites that now push .name.clone() (the
batch handling around r.is_error and the execute_tool call), and then change the
had_errors check used by crate::auto_skill::should_create_skill and any
subsequent usages (e.g., generate_skill_content) to inspect the stored booleans
or extract names-only where needed.
---
Nitpick comments:
In `@mc/crates/mc-core/src/runtime.rs`:
- Around line 678-679: The code computing skills_dir via
std::env::var_os("HOME") is Unix-specific and will be None on Windows; update
the logic that sets skills_dir in runtime.rs (the variable named skills_dir) to
use a cross-platform config directory (e.g., use dirs::config_dir() and join
".magic-code/skills") or at minimum fall back to std::env::var_os("USERPROFILE")
when HOME is missing; if you choose dirs::config_dir(), add the dirs crate to
Cargo.toml and ensure you handle Option results and join the path similarly to
the original code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd66eb44-d981-4720-b33d-7936eafb0f46
📒 Files selected for processing (3)
mc/crates/mc-core/src/auto_skill.rsmc/crates/mc-core/src/fts.rsmc/crates/mc-core/src/runtime.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- mc/crates/mc-core/src/fts.rs
- mc/crates/mc-core/src/auto_skill.rs
1. Auto-skill creation (mc-core/src/auto_skill.rs): - After complex turns (≥6 tool calls, no errors), auto-generates skill file - Saves to ~/.config/magic-code/skills/auto/ - Won't overwrite existing skills - Notifies user via TextDelta event 2. Cross-session FTS search (mc-core/src/fts.rs): - /search-all <query> searches across all saved session files - Returns session name, timestamp, and context snippet - Scans up to 100 most recent sessions, max 20 results - Case-insensitive matching 3. Session.created_at field added for timestamp tracking 201 tests (4 new), 0 warnings.
1. auto_skill: atomic create_new(true) instead of exists() check (TOCTOU fix) 2. auto_skill: add #[must_use] to save_auto_skill 3. auto_skill: add timestamp to filename (collision fix) 4. fts: safe byte slicing via char_indices (unicode panic fix) 5. fts: search Thinking, ToolUse, ToolResult blocks too 201 tests, 0 warnings.
kienbui1995
force-pushed
the
feat/auto-skill-fts
branch
from
29507f4 to
604b2b9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two features adapted from Hermes Agent for coding context:
1. Auto-skill creation
After complex successful turns (≥6 tool calls), agent auto-generates a reusable skill file:
Next time agent encounters similar task, it can load the skill. Won't overwrite existing skills.
2. Cross-session FTS search
/search-all fix auth timeout— searches ALL saved sessions:Scoped to coding agent needs — no messaging gateway, no user modeling.
201 tests, 0 warnings.
Summary by CodeRabbit
New Features
Tests