feat: session name templates, session resilience, and REPL improvements#134
Merged
Conversation
…ommand Implements GitHub issue #116: - Add expand_session_variables() and sanitize_session_name() utilities - Simplify apply_default_session() — remove all colon-delimited parsing - Integrate variable expansion into agent default session - Add .reset REPL command to re-initialize session with re-expanded variables - Update config.example.yaml documentation Session name templates now support $VAR and ${VAR} syntax with special computed variables $GIT_BRANCH and $GIT_PATH, plus any environment variable. Names are sanitized (/ → -, space → _, strip leading/trailing - and _).
Adds expand_session_variables_with() that accepts extra context variables. When an agent is active, $AGENT_NAME resolves to the agent's name in agent_default_session templates and .reset re-expansion. Extra vars take precedence over env vars but not over the built-in computed variables (GIT_BRANCH, GIT_PATH). Includes 4 new unit tests.
use_agent() only checked the per-agent front-matter field, never falling back to Config.agent_default_session. Similarly, reset_session() had the same gap. Now both check per-agent first, then fall back to the global config value. apply_default_session() is unaffected since it only runs for REPL/CMD modes (the agent path goes through use_agent instead).
…r readability - Apply consistent method-chaining indentation across session template processing. - Simplify function signatures and line breaks in `expand_session_variables_with`. - Update comments and formatting for cleaner code structure and readability. Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSession-name template expansion/sanitization added; Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant REPL as REPL
participant Config as Config
participant SessionUtil as SessionUtil
participant Git as Git
User->>REPL: ".reset repl"
REPL->>Config: reset_session()
Config->>Config: drop current session / exit agent
Config->>SessionUtil: expand_session_variables_with(template, {AGENT_NAME})
SessionUtil->>Git: git rev-parse --abbrev-ref / --show-prefix (if referenced)
Git-->>SessionUtil: branch/path (or empty)
SessionUtil-->>Config: expanded template
Config->>SessionUtil: sanitize_session_name(expanded)
SessionUtil-->>Config: sanitized name
Config->>Config: use_session(sanitized name)
Config-->>REPL: session activated
REPL-->>User: new session established
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
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 `@src/config/mod.rs`:
- Around line 1242-1266: When resetting you drop self.session before recomputing
the new session template, so sessions loaded from disk that embed agent context
lose $AGENT_NAME and agent_default_session; fix by capturing the agent-backed
context (agent name and per-agent default template) from the existing session
(via the same logic used by state() / extract_agent()) before calling
self.session.take(), then use that captured agent name/template as the
extra_vars and template source when calling
session_name::expand_session_variables_with and
session_name::sanitize_session_name (falling back to self.agent_default_session
only if neither self.agent nor the captured session-backed template exist) so
reset preserves agent-backed session variables.
In `@src/utils/session_name.rs`:
- Around line 69-80: sanitize_session_name currently only replaces '/' and ' '
allowing backslashes, colons, path-traversal sequences and other
Windows-reserved characters to slip through; update sanitize_session_name to
also replace backslash ('\\'), colon (':'), all unicode whitespace variants (use
is_whitespace()), and Windows-reserved characters (< > : " / \\ | ? *), collapse
consecutive separator characters into a single '-' or '_' as appropriate, remove
any leading/trailing separators and strip any occurrences of path-traversal
segments like ".." (or replace dots adjacent to separators), and ensure the
function returns a non-empty safe fallback (e.g., "session") so
Config::session_file() cannot be given an escaped absolute or invalid filename.
- Around line 130-136: Tests like test_expand_env_var mutate process-wide
environment without synchronization, causing races; introduce a shared static
mutex (e.g., ENV_MUTEX using once_cell::sync::Lazy<Mutex<()>> or lazy_static!
with a Mutex<()>) and wrap every env set/remove sequence for
expand_session_variables and the other affected tests behind a lock. Consolidate
setup/teardown into a small helper (e.g., a with_env_var or set_temp_env that
acquires ENV_MUTEX, sets the var, runs the assertion/closure, and then removes
the var) and update test_expand_env_var and the other listed tests to use that
helper so environment mutation is serialized and always cleaned up.
- Around line 97-117: Convert git_branch() and git_path() into async functions
that use tokio::process::Command (e.g., async fn git_branch() -> String / async
fn git_path() -> String), replace blocking Command::output() calls with
.output().await, handle the Result/Status as before and return the computed
String, then call and await these functions at the async boundary (where
use_agent() or session expansion happens) and pass the resolved strings into
expand_session_variables() instead of calling blocking helpers inside the
formatter.
🪄 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: 290395f0-47cd-47a7-b7f6-2665c44d36e2
📒 Files selected for processing (5)
config.example.yamlsrc/config/mod.rssrc/repl/mod.rssrc/utils/mod.rssrc/utils/session_name.rs
Comment on lines
+1242
to
+1266
| // Discard the current session without saving | ||
| if let Some(session) = self.session.take() { | ||
| drop(session); | ||
| self.discontinuous_last_message(); | ||
| } | ||
| if let Some(agent) = self.agent.as_mut() { | ||
| agent.exit_session(); | ||
| } | ||
|
|
||
| // Re-create a session with freshly-expanded variables | ||
| let new_session_name = if let Some(agent) = &self.agent { | ||
| let extra_vars = std::collections::HashMap::from([("AGENT_NAME", agent.name())]); | ||
| // Per-agent front-matter first, then global config fallback | ||
| let template = agent | ||
| .agent_default_session() | ||
| .map(|s| s.to_string()) | ||
| .or_else(|| self.agent_default_session.clone()); | ||
| template | ||
| .map(|v| { | ||
| session_name::sanitize_session_name( | ||
| &session_name::expand_session_variables_with(&v, &extra_vars), | ||
| ) | ||
| }) | ||
| .filter(|v| !v.is_empty()) | ||
| } else { |
Contributor
There was a problem hiding this comment.
Preserve agent-backed session context when resetting.
state() and extract_agent() already treat a loaded session as an agent context even when self.agent is None. Here the session is dropped first, and the recomputation path only checks self.agent, so .reset loses $AGENT_NAME and agent_default_session for sessions loaded from disk with embedded agent data. Capture the session-backed agent/template source before self.session.take() and use that during recomputation.
🔧 Sketch of the fix
+ let session_agent = self
+ .session
+ .as_ref()
+ .filter(|session| session.agent_name().is_some())
+ .map(|session| session.to_agent());
+
// Discard the current session without saving
if let Some(session) = self.session.take() {
drop(session);
self.discontinuous_last_message();
}
@@
- let new_session_name = if let Some(agent) = &self.agent {
+ let new_session_name = if let Some(agent) =
+ self.agent.as_ref().or(session_agent.as_ref())
+ {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config/mod.rs` around lines 1242 - 1266, When resetting you drop
self.session before recomputing the new session template, so sessions loaded
from disk that embed agent context lose $AGENT_NAME and agent_default_session;
fix by capturing the agent-backed context (agent name and per-agent default
template) from the existing session (via the same logic used by state() /
extract_agent()) before calling self.session.take(), then use that captured
agent name/template as the extra_vars and template source when calling
session_name::expand_session_variables_with and
session_name::sanitize_session_name (falling back to self.agent_default_session
only if neither self.agent nor the captured session-backed template exist) so
reset preserves agent-backed session variables.
Comment on lines
+97
to
+117
| fn git_branch() -> String { | ||
| Command::new("git") | ||
| .args(["rev-parse", "--abbrev-ref", "HEAD"]) | ||
| .output() | ||
| .ok() | ||
| .filter(|o| o.status.success()) | ||
| .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()) | ||
| .unwrap_or_default() | ||
| } | ||
|
|
||
| fn git_path() -> String { | ||
| Command::new("git") | ||
| .args(["rev-parse", "--show-prefix"]) | ||
| .output() | ||
| .ok() | ||
| .filter(|o| o.status.success()) | ||
| .map(|o| { | ||
| let s = String::from_utf8_lossy(&o.stdout).trim().to_string(); | ||
| s.trim_end_matches('/').to_string() | ||
| }) | ||
| .unwrap_or_default() |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '97,117p' src/utils/session_name.rs
rg -n 'expand_session_variables(_with)?\(' src/config/mod.rsRepository: dobesv/harnx
Length of output: 1055
🏁 Script executed:
# Find where git_branch and git_path are called
rg -n 'git_branch\(\)|git_path\(\)' src/
# Check the signatures and context of expand_session_variables functions
rg -n 'fn expand_session_variables' src/
# Check the signatures of use_agent, reset_session, apply_default_session
rg -n 'fn (use_agent|reset_session|apply_default_session)' src/Repository: dobesv/harnx
Length of output: 1113
🏁 Script executed:
# Check the full expand_session_variables function
sed -n '14,95p' src/utils/session_name.rs
# Check the actual callsites in config/mod.rs
sed -n '1560,1580p' src/config/mod.rs
sed -n '1230,1250p' src/config/mod.rs
sed -n '1675,1695p' src/config/mod.rs
sed -n '250,260p' src/utils/session_name.rsRepository: dobesv/harnx
Length of output: 5360
🏁 Script executed:
# Check context around line 1262 and 1595 (likely in use_agent)
sed -n '1560,1600p' src/config/mod.rs
# Check context around line 1275 (likely in another function)
sed -n '1268,1285p' src/config/mod.rs
# Check context around line 1702 (apply_default_session)
sed -n '1695,1715p' src/config/mod.rsRepository: dobesv/harnx
Length of output: 3146
🏁 Script executed:
# Check for any tokio::task::block_in_place usage
rg 'block_in_place' src/
# Check for std::process usage in async contexts
rg 'std::process' src/utils/session_name.rs
# Verify the Command import
rg 'use.*Command' src/utils/session_name.rsRepository: dobesv/harnx
Length of output: 751
Move git_branch() and git_path() to async using tokio::process::Command and pass resolved values to formatter.
These helpers call Command::output() (blocking I/O) directly, which blocks the async executor when invoked from use_agent() during session name expansion. Per coding guidelines, all I/O operations must use async via Tokio with async fn and .await. Replace std::process::Command with tokio::process::Command and convert these functions to async, then resolve the values at the async boundary before calling expand_session_variables().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/session_name.rs` around lines 97 - 117, Convert git_branch() and
git_path() into async functions that use tokio::process::Command (e.g., async fn
git_branch() -> String / async fn git_path() -> String), replace blocking
Command::output() calls with .output().await, handle the Result/Status as before
and return the computed String, then call and await these functions at the async
boundary (where use_agent() or session expansion happens) and pass the resolved
strings into expand_session_variables() instead of calling blocking helpers
inside the formatter.
Comment on lines
+130
to
+136
| fn test_expand_env_var() { | ||
| std::env::set_var("HARNX_TEST_SESSION_VAR", "hello"); | ||
| assert_eq!( | ||
| expand_session_variables("prefix-$HARNX_TEST_SESSION_VAR-suffix"), | ||
| "prefix-hello-suffix" | ||
| ); | ||
| std::env::remove_var("HARNX_TEST_SESSION_VAR"); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -H '^Cargo.toml$' -x sed -n '1,80p' {}
rg -n 'std::env::(set_var|remove_var)\(' src/utils/session_name.rsRepository: dobesv/harnx
Length of output: 3591
🏁 Script executed:
# Check the full test file structure and look for test synchronization patterns
wc -l src/utils/session_name.rs
head -20 src/utils/session_name.rs
tail -50 src/utils/session_name.rsRepository: dobesv/harnx
Length of output: 2770
🏁 Script executed:
# Search for mutex or test synchronization patterns in the codebase
rg -n 'mutex|Mutex|serial|sync|lock' src/utils/session_name.rs -i
rg -n '#\[.*test.*\]' src/utils/session_name.rsRepository: dobesv/harnx
Length of output: 408
🌐 Web query:
Rust 2024 edition std::env::set_var unsafe requirement
💡 Result:
In the Rust 2024 edition, std::env::set_var is marked as an unsafe function, requiring it to be called within an unsafe block. This change was made because calling set_var (or remove_var) can be unsound in multithreaded programs on certain platforms (non-Windows), due to the process environment not being handled in a thread-safe manner. Previously, these were safe functions, but it was determined they should have been unsafe from the start. Key details: - Safe in single-threaded programs or always on Windows. - In multi-threaded non-Windows programs, unsafe unless no other threads access the environment (practically impossible to guarantee, so avoid entirely). - Edition-specific: Allowed without unsafe in pre-2024 editions; requires unsafe in 2024+. - Migration: Use cargo fix --edition to auto-wrap calls in unsafe blocks with a TODO comment for auditing. This was implemented via PR #124636 (merged for 1.80.0) and documented in the Rust Edition Guide.
Citations:
- 1: Make
std::env::{set_var, remove_var}unsafe in edition 2024 rust-lang/rust#124636 - 2: https://doc.rust-lang.org/edition-guide/rust-2024/newly-unsafe-functions.html
- 3: https://doc.rust-lang.org/beta/edition-guide/rust-2024/newly-unsafe-functions.html
- 4: Tracking Issue for Rust 2024: Make
std::env::{set_var, remove_var}unsafe rust-lang/rust#124866 - 5: https://doc.rust-lang.org/stable/std/env/fn.set_var.html
Use a shared mutex to prevent test race conditions when mutating environment variables.
These tests call std::env::set_var / remove_var without synchronization. Since tests run in parallel by default, concurrent mutations create race conditions. Wrap these env mutations behind a shared mutex and consolidate the setup/teardown logic into a single function.
Note: The crate targets edition 2021 (not 2024), so unsafe is not required for these APIs; the issue is purely about test isolation and race prevention.
Affected tests: lines 130-136, 140-146, 150-156, 175-182, 235-239, 261-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/session_name.rs` around lines 130 - 136, Tests like
test_expand_env_var mutate process-wide environment without synchronization,
causing races; introduce a shared static mutex (e.g., ENV_MUTEX using
once_cell::sync::Lazy<Mutex<()>> or lazy_static! with a Mutex<()>) and wrap
every env set/remove sequence for expand_session_variables and the other
affected tests behind a lock. Consolidate setup/teardown into a small helper
(e.g., a with_env_var or set_temp_env that acquires ENV_MUTEX, sets the var,
runs the assertion/closure, and then removes the var) and update
test_expand_env_var and the other listed tests to use that helper so environment
mutation is serialized and always cleaned up.
Change the use_tools field from Option<String> to Option<Vec<String>>
across all config structures (Config, Agent, AgentFrontMatter, Session).
Add a custom serde deserializer (deserialize_use_tools) that accepts both:
- A proper YAML list: ['fs:all', 'bash_exec']
- A comma-separated string: 'fs:all,bash_exec' (backward compatible)
This reuses the existing ToolsetValue enum and normalize_toolset_value()
function already used by toolsets deserialization.
Updated consumers:
- Config::active_tool_names() - iterate vec instead of split(',')
- Config::select_tools() - iterate vec instead of split(',')
- Config::update() (.set use_tools) - parse comma-separated input to vec
- Config::info() - join vec for display
- CLI --tool flag - push to vec directly
- REPL .use tool / .drop tool - push/remove from vec directly
- Session::export() - serialize as JSON array
- Session::render() - join for display
- Environment variable reading - split on comma
- Tab completion for .drop tool - iterate vec
Updated documentation:
- docs/agent-guide.md - YAML list syntax in examples and reference table
- docs/configuration-guide.md - updated descriptions
- config.example.yaml - already used list format (no change needed)
- config.agent.example.md - already used list format (no change needed)
Updated tests to assert against Vec<String> instead of comma-joined String.
Three changes: 1. call_chat_completions_streaming: On abort, return partial text as Ok instead of bailing. On send error with partial text, return Ok with the partial text instead of Err. This lets callers save whatever was streamed before the interruption. 2. ask_inner (REPL) error paths: Call after_chat_completion with empty output before returning Err, so the user's input message is saved to the session even when the LLM call fails. 3. start_directive_inner (CMD) error paths: Same — call after_chat_completion before returning, preserving the user's input. With these changes, if the agent is interrupted (Ctrl+C) or the provider returns an error (rate limit, etc.), the user's message and any partial assistant output are preserved in the session. The user can then say 'continue' to resume.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/mod.rs (1)
1744-1770:⚠️ Potential issue | 🟠 Major
<server>:allstill breaks when tools are renamed.This branch ultimately resolves matches from
ToolDeclaration.name, but renamed MCP tools expose canonical names likeRead/Write, notfs_*. Withrename_toolsconfigured,fs:allresolves to an empty tool set here. Match on the underlying server/MCP identifier instead, and mirror the same fix inactive_tool_names().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/mod.rs` around lines 1744 - 1770, The ":all" branch currently filters declaration_names by a prefix built from the user-visible names (format!("{server}_")), which breaks when tools are renamed; update that branch to match declarations by their underlying server/MCP identifier instead of ToolDeclaration.name (e.g., iterate declarations from tool_declarations_for_use_tools and select those where declaration.server_id / declaration.mcp_id equals the parsed server, then collect their ToolDeclaration.name into tool_names). Apply the same change to the active_tool_names() implementation so both paths use the MCP/server identifier to resolve "<server>:all".
♻️ Duplicate comments (1)
src/config/mod.rs (1)
1268-1313:⚠️ Potential issue | 🟠 MajorPreserve session-backed agent context before
.reset.This recomputes the template only from
self.agentafterself.session.take(). A session loaded from disk with an embeddedagent_nametherefore loses$AGENT_NAMEand any per-agent default-session template on reset, even thoughstate()/extract_agent()treat that session as agent context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/mod.rs` around lines 1268 - 1313, In reset_session, preserve the agent-derived context before discarding self.session: capture the effective agent name and per-agent default-session template (from either self.agent or from the taken session's embedded agent info) before calling self.session.take(); then use those captured values when computing new_session_name so $AGENT_NAME and per-agent templates are not lost. Update reset_session to read agent name/template into local variables (e.g., captured_agent_name, captured_agent_template) before the session is dropped and use those in the session_name::expand_session_variables_with / agent_default_session logic, then call self.use_session(Some(&name)) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config.agent.example.md`:
- Line 8: Update the inline example for use_tools to remove the legacy
"bash_exec" string and show the canonical Bash tool name used elsewhere (e.g.,
change "fs:all,bash_exec" to "fs:all,bash:exec"); edit the comment around the
use_tools example so it matches the canonical tool naming and will copy-paste
correctly, referencing the use_tools key in the file.
In `@docs/agent-guide.md`:
- Around line 26-28: In the docs examples where the YAML use_tools list contains
"bash_exec" (e.g., the block with use_tools: - fs:all - bash_exec), replace the
legacy "bash_exec" token with the repository's canonical Bash tool identifier
used elsewhere in docs (the renamed Bash declaration), and update the other
occurrence noted (the snippet around the second use_tools instance) so both
examples use the same canonical Bash tool name; ensure you only change the
identifier string and keep the surrounding YAML structure (use_tools, fs:all)
intact.
In `@src/config/mod.rs`:
- Around line 1970-1972: The file fails rustfmt: run cargo fmt to reformat the
iterator chain and remove the stray blank line; specifically adjust the iterator
expression that currently ends with .into_iter().map(|s| (s, None)).collect() so
it conforms to rustfmt (or simplify by using iter/map as appropriate) and delete
the extra blank line near the function/tool_declarations_for_use_tools()
declaration so the file passes cargo fmt --check.
In `@src/config/session.rs`:
- Around line 163-165: The block setting data["use_tools"] is not formatted per
rustfmt; reformat the expression that constructs the serde_json::Value::Array
from use_tools (the line with data["use_tools"] =
serde_json::Value::Array(use_tools.into_iter().map(serde_json::Value::String).collect(),);)
to match rustfmt style and then run cargo fmt (or cargo fmt --all) so the file
passes cargo fmt --check; ensure the call to serde_json::Value::Array and the
mapped collect() are line-wrapped and indented according to rustfmt rules.
In `@src/main.rs`:
- Around line 153-160: The current code updates global config.use_tools, which
ignores --tool for already-loaded scopes; instead read the active scope's tool
list via extract_agent().use_tools() (or call the agent accessor used in run()),
merge in cli.tool avoiding duplicates, and persist it using the agent's setter
set_use_tools(...) rather than writing to config.write().use_tools; update the
block that currently references config.read()/config.write() to use
extract_agent().use_tools() as the base and call set_use_tools(merged_tools) to
apply the change to the active scope.
---
Outside diff comments:
In `@src/config/mod.rs`:
- Around line 1744-1770: The ":all" branch currently filters declaration_names
by a prefix built from the user-visible names (format!("{server}_")), which
breaks when tools are renamed; update that branch to match declarations by their
underlying server/MCP identifier instead of ToolDeclaration.name (e.g., iterate
declarations from tool_declarations_for_use_tools and select those where
declaration.server_id / declaration.mcp_id equals the parsed server, then
collect their ToolDeclaration.name into tool_names). Apply the same change to
the active_tool_names() implementation so both paths use the MCP/server
identifier to resolve "<server>:all".
---
Duplicate comments:
In `@src/config/mod.rs`:
- Around line 1268-1313: In reset_session, preserve the agent-derived context
before discarding self.session: capture the effective agent name and per-agent
default-session template (from either self.agent or from the taken session's
embedded agent info) before calling self.session.take(); then use those captured
values when computing new_session_name so $AGENT_NAME and per-agent templates
are not lost. Update reset_session to read agent name/template into local
variables (e.g., captured_agent_name, captured_agent_template) before the
session is dropped and use those in the
session_name::expand_session_variables_with / agent_default_session logic, then
call self.use_session(Some(&name)) as before.
🪄 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: e06245df-0c04-4681-96ae-35d635e8f6fc
📒 Files selected for processing (9)
config.agent.example.mdconfig.example.yamldocs/agent-guide.mddocs/configuration-guide.mdsrc/config/agent.rssrc/config/mod.rssrc/config/session.rssrc/main.rssrc/repl/mod.rs
✅ Files skipped from review due to trivial changes (1)
- docs/configuration-guide.md
| temperature: null # Set default temperature parameter, range (0, 1) | ||
| top_p: null # Set default top-p parameter | ||
| use_tools: null # Which MCP tools to use (e.g. 'fs:all,bash_exec') | ||
| use_tools: # Which MCP tools to use (e.g. 'fs:all,bash_exec') |
Contributor
There was a problem hiding this comment.
Refresh the inline use_tools example.
The YAML list is current, but the comment still advertises the legacy bash_exec string form. Copy-pasting that value will not match the canonical Bash tool name used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config.agent.example.md` at line 8, Update the inline example for use_tools
to remove the legacy "bash_exec" string and show the canonical Bash tool name
used elsewhere (e.g., change "fs:all,bash_exec" to "fs:all,bash:exec"); edit the
comment around the use_tools example so it matches the canonical tool naming and
will copy-paste correctly, referencing the use_tools key in the file.
Comment on lines
+26
to
+28
| use_tools: | ||
| - fs:all | ||
| - bash_exec |
Contributor
There was a problem hiding this comment.
Replace bash_exec in the examples.
The guide now documents canonical tool names/lists, but these snippets still use the old bash_exec identifier. Copy-pasting them into use_tools will not match the renamed Bash declaration.
Also applies to: 336-338
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/agent-guide.md` around lines 26 - 28, In the docs examples where the
YAML use_tools list contains "bash_exec" (e.g., the block with use_tools: -
fs:all - bash_exec), replace the legacy "bash_exec" token with the repository's
canonical Bash tool identifier used elsewhere in docs (the renamed Bash
declaration), and update the other occurrence noted (the snippet around the
second use_tools instance) so both examples use the same canonical Bash tool
name; ensure you only change the identifier string and keep the surrounding YAML
structure (use_tools, fs:all) intact.
When repl_default_session is configured, the REPL already has an active session. Running .agent would set the agent on config but then fail at use_session() with 'Already in a session' because the old REPL session was still active. Now use_agent() exits any existing session before creating the agent's session, matching the pattern already used by the switch-agent code path in main.rs and repl/mod.rs. Ref: #116
- Skip rendering dots in spinner when message is empty. - Update spinner initialization to use an empty message in affected components (`stream.rs` and `common.rs`). Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
This reverts commit 32b56bc.
When Session::load() deserializes a saved session YAML that has no use_tools field (common for sessions saved before use_tools was added to the agent, or older sessions), the session's use_tools ends up as None. This caused .info tools to show all tools as disabled (○) even though tools actually worked via the select_tools() fallback path. Fix: when loading a session that belongs to an agent, if the session has no use_tools, re-apply the agent's current use_tools. This mirrors what Session::new() already does via set_agent().
Follows the existing pattern of subcommand-style REPL commands (.empty session, .rebuild rag, .info session, etc.). Bare '.reset' now prints usage guidance.
- Streamlined `.drop tool` iterator logic for clarity. - Reformatted session data mapping for consistent chaining style. Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
dobesv
changed the title
…00 tokens Signed-off-by: Dobes Vandermeer <dobes.vandermeer@newsela.com>
Address CodeRabbit review comments on PR #134: 1. sanitize_session_name now handles: - Backslashes (treated same as forward slashes → '-') - All unicode whitespace (→ '_') - Windows-reserved characters (<>:"|?*) → removed - Path-traversal segments (..) → removed - Consecutive separators → collapsed - Empty results → fallback to 'session' 2. --tool CLI flag now reads from the active scope (extract_agent().use_tools()) and writes via set_use_tools() instead of directly accessing config.use_tools. This ensures --tool works correctly with --agent and --session.
This was referenced Apr 4, 2026
Merged
This was referenced Apr 12, 2026
Merged
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.
Overview
Session names now support variable templates, sessions are more resilient to interrupts and errors, and several REPL/agent-switching bugs are fixed.
Features
agent_default_sessionandrepl_default_sessionsupport$VAR/${VAR}env vars plus computed variables ($GIT_BRANCH,$GIT_PATH,$AGENT_NAME). Names are deterministically sanitized for filesystem safety.$AGENT_NAMEvariable — resolves to the active agent name in session templates, with extra-vars support and precedence rules..reset replcommand — renamed from.reset; re-expands session name variables (e.g. after switching git branches). Follows the subcommand pattern of.empty session,.rebuild rag, etc.continueto resume.Bug Fixes
agent_default_sessionwas never applied —use_agent()andreset_session()now fall back to the global config value when the per-agent front-matter field is absent..agentwith an active REPL session no longer fails with "Already in a session". Fixes Default session calculation #116.use_toolswhen loading a saved session — sessions saved beforeuse_toolswas configured (or withuse_tools: None) now pick up the agent's currentuse_toolson load, fixing.info toolsshowing all tools as disabled.Refactors
use_toolsis now a YAML list — changed fromOption<String>toOption<Vec<String>>across Config, Agent, AgentFrontMatter, and Session. A custom serde deserializer accepts both YAML lists and legacy comma-separated strings for backward compatibility.Summary by CodeRabbit
New Features
.reset replcommand to clear and reinitialize REPL sessions.Configuration & Improvements
Documentation