Skip to content

Commit fdb4900

Browse files
authored
fix(agent): context slashcommand (#2494)
1 parent 9df8471 commit fdb4900

File tree

6 files changed

+182
-23
lines changed

6 files changed

+182
-23
lines changed

crates/chat-cli/src/cli/agent/hook.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ impl Display for HookTrigger {
3232
}
3333
}
3434

35+
#[derive(Debug, Clone, Deserialize, Eq, PartialEq, Hash)]
36+
pub enum Source {
37+
Agent,
38+
Session,
39+
}
40+
41+
impl Default for Source {
42+
fn default() -> Self {
43+
Self::Agent
44+
}
45+
}
46+
3547
#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, JsonSchema, Hash)]
3648
pub struct Hook {
3749
/// The command to run when the hook is triggered
@@ -48,15 +60,20 @@ pub struct Hook {
4860
/// How long the hook output is cached before it will be executed again
4961
#[serde(default = "Hook::default_cache_ttl_seconds")]
5062
pub cache_ttl_seconds: u64,
63+
64+
#[schemars(skip)]
65+
#[serde(default, skip_serializing)]
66+
pub source: Source,
5167
}
5268

5369
impl Hook {
54-
pub fn new(command: String) -> Self {
70+
pub fn new(command: String, source: Source) -> Self {
5571
Self {
5672
command,
5773
timeout_ms: Self::default_timeout_ms(),
5874
max_output_size: Self::default_max_output_size(),
5975
cache_ttl_seconds: Self::default_cache_ttl_seconds(),
76+
source,
6077
}
6178
}
6279

crates/chat-cli/src/cli/agent/legacy/hooks.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ impl From<LegacyHook> for Option<Hook> {
8080
timeout_ms: value.timeout_ms,
8181
max_output_size: value.max_output_size,
8282
cache_ttl_seconds: value.cache_ttl_seconds,
83+
source: Default::default(),
8384
})
8485
}
8586
}

crates/chat-cli/src/cli/chat/cli/context.rs

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::cli::chat::consts::{
1414
AGENT_FORMAT_HOOKS_DOC_URL,
1515
CONTEXT_FILES_MAX_SIZE,
1616
};
17+
use crate::cli::chat::context::ContextFilePath;
1718
use crate::cli::chat::token_counter::TokenCounter;
1819
use crate::cli::chat::util::drop_matched_context_files;
1920
use crate::cli::chat::{
@@ -82,26 +83,37 @@ impl ContextSubcommand {
8283

8384
match self {
8485
Self::Show { expand } => {
85-
let profile_context_files = HashSet::<(String, String)>::new();
86+
// the bool signifies if the resources is temporary (i.e. is it session based as
87+
// opposed to agent based)
88+
let mut profile_context_files = HashSet::<(String, String, bool)>::new();
89+
90+
let (agent_owned_list, session_owned_list) = context_manager
91+
.paths
92+
.iter()
93+
.partition::<Vec<_>, _>(|p| matches!(**p, ContextFilePath::Agent(_)));
94+
8695
execute!(
8796
session.stderr,
8897
style::SetAttribute(Attribute::Bold),
8998
style::SetForegroundColor(Color::Magenta),
90-
style::Print(format!("\n👤 Agent ({}):\n", context_manager.current_profile)),
99+
style::Print(format!("👤 Agent ({}):\n", context_manager.current_profile)),
91100
style::SetAttribute(Attribute::Reset),
92101
)?;
93102

94-
if context_manager.paths.is_empty() {
103+
if agent_owned_list.is_empty() {
95104
execute!(
96105
session.stderr,
97106
style::SetForegroundColor(Color::DarkGrey),
98107
style::Print(" <none>\n\n"),
99108
style::SetForegroundColor(Color::Reset)
100109
)?;
101110
} else {
102-
for path in &context_manager.paths {
103-
execute!(session.stderr, style::Print(format!(" {} ", path)))?;
104-
if let Ok(context_files) = context_manager.get_context_files_by_path(os, path).await {
111+
for path in &agent_owned_list {
112+
execute!(session.stderr, style::Print(format!(" {} ", path.get_path_as_str())))?;
113+
if let Ok(context_files) = context_manager
114+
.get_context_files_by_path(os, path.get_path_as_str())
115+
.await
116+
{
105117
execute!(
106118
session.stderr,
107119
style::SetForegroundColor(Color::Green),
@@ -112,6 +124,48 @@ impl ContextSubcommand {
112124
)),
113125
style::SetForegroundColor(Color::Reset)
114126
)?;
127+
profile_context_files
128+
.extend(context_files.into_iter().map(|(path, content)| (path, content, false)));
129+
}
130+
execute!(session.stderr, style::Print("\n"))?;
131+
}
132+
execute!(session.stderr, style::Print("\n"))?;
133+
}
134+
135+
execute!(
136+
session.stderr,
137+
style::SetAttribute(Attribute::Bold),
138+
style::SetForegroundColor(Color::Magenta),
139+
style::Print("💬 Session (temporary):\n"),
140+
style::SetAttribute(Attribute::Reset),
141+
)?;
142+
143+
if session_owned_list.is_empty() {
144+
execute!(
145+
session.stderr,
146+
style::SetForegroundColor(Color::DarkGrey),
147+
style::Print(" <none>\n\n"),
148+
style::SetForegroundColor(Color::Reset)
149+
)?;
150+
} else {
151+
for path in &session_owned_list {
152+
execute!(session.stderr, style::Print(format!(" {} ", path.get_path_as_str())))?;
153+
if let Ok(context_files) = context_manager
154+
.get_context_files_by_path(os, path.get_path_as_str())
155+
.await
156+
{
157+
execute!(
158+
session.stderr,
159+
style::SetForegroundColor(Color::Green),
160+
style::Print(format!(
161+
"({} match{})",
162+
context_files.len(),
163+
if context_files.len() == 1 { "" } else { "es" }
164+
)),
165+
style::SetForegroundColor(Color::Reset)
166+
)?;
167+
profile_context_files
168+
.extend(context_files.into_iter().map(|(path, content)| (path, content, true)));
115169
}
116170
execute!(session.stderr, style::Print("\n"))?;
117171
}
@@ -129,7 +183,7 @@ impl ContextSubcommand {
129183
let total = profile_context_files.len();
130184
let total_tokens = profile_context_files
131185
.iter()
132-
.map(|(_, content)| TokenCounter::count_tokens(content))
186+
.map(|(_, content, _)| TokenCounter::count_tokens(content))
133187
.sum::<usize>();
134188
execute!(
135189
session.stderr,
@@ -144,11 +198,12 @@ impl ContextSubcommand {
144198
style::SetAttribute(Attribute::Reset)
145199
)?;
146200

147-
for (filename, content) in &profile_context_files {
201+
for (filename, content, is_temporary) in &profile_context_files {
148202
let est_tokens = TokenCounter::count_tokens(content);
203+
let icon = if *is_temporary { "💬" } else { "👤" };
149204
execute!(
150205
session.stderr,
151-
style::Print(format!("👤 {} ", filename)),
206+
style::Print(format!("{} {} ", icon, filename)),
152207
style::SetForegroundColor(Color::DarkGrey),
153208
style::Print(format!("(~{} tkns)\n", est_tokens)),
154209
style::SetForegroundColor(Color::Reset),
@@ -167,7 +222,10 @@ impl ContextSubcommand {
167222
execute!(session.stderr, style::Print(format!("{}\n\n", "▔".repeat(3))),)?;
168223
}
169224

170-
let mut files_as_vec = profile_context_files.iter().cloned().collect::<Vec<_>>();
225+
let mut files_as_vec = profile_context_files
226+
.iter()
227+
.map(|(path, content, _)| (path.clone(), content.clone()))
228+
.collect::<Vec<_>>();
171229
let dropped_files = drop_matched_context_files(&mut files_as_vec, CONTEXT_FILES_MAX_SIZE).ok();
172230

173231
execute!(
@@ -240,7 +298,8 @@ impl ContextSubcommand {
240298
execute!(
241299
session.stderr,
242300
style::SetForegroundColor(Color::Green),
243-
style::Print(format!("\nAdded {} path(s) to context.\n\n", paths.len())),
301+
style::Print(format!("\nAdded {} path(s) to context.\n", paths.len())),
302+
style::Print("Note: Context modifications via slash command is temporary.\n\n"),
244303
style::SetForegroundColor(Color::Reset)
245304
)?;
246305
},
@@ -259,6 +318,7 @@ impl ContextSubcommand {
259318
session.stderr,
260319
style::SetForegroundColor(Color::Green),
261320
style::Print(format!("\nRemoved {} path(s) from context.\n\n", paths.len(),)),
321+
style::Print("Note: Context modifications via slash command is temporary.\n\n"),
262322
style::SetForegroundColor(Color::Reset)
263323
)?;
264324
},
@@ -276,7 +336,8 @@ impl ContextSubcommand {
276336
execute!(
277337
session.stderr,
278338
style::SetForegroundColor(Color::Green),
279-
style::Print("\nCleared context\n\n"),
339+
style::Print("\nCleared context\n"),
340+
style::Print("Note: Context modifications via slash command is temporary.\n\n"),
280341
style::SetForegroundColor(Color::Reset)
281342
)?;
282343
},

crates/chat-cli/src/cli/chat/context.rs

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use eyre::{
99
use glob::glob;
1010
use serde::{
1111
Deserialize,
12+
Deserializer,
1213
Serialize,
14+
Serializer,
1315
};
1416

1517
use super::consts::CONTEXT_FILES_MAX_SIZE;
@@ -23,14 +25,77 @@ use crate::cli::chat::ChatError;
2325
use crate::cli::chat::cli::hooks::HookExecutor;
2426
use crate::os::Os;
2527

28+
#[derive(Debug, Clone)]
29+
pub enum ContextFilePath {
30+
/// Signifies that the path is brought in from the agent config
31+
Agent(String),
32+
/// Signifies that the path is brought in via /context add
33+
Session(String),
34+
}
35+
36+
impl Serialize for ContextFilePath {
37+
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
38+
where
39+
S: Serializer,
40+
{
41+
match self {
42+
ContextFilePath::Agent(path) => path.serialize(serializer),
43+
ContextFilePath::Session(_) => Err(serde::ser::Error::custom("Session paths are not serialized")),
44+
}
45+
}
46+
}
47+
48+
impl<'de> Deserialize<'de> for ContextFilePath {
49+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
50+
where
51+
D: Deserializer<'de>,
52+
{
53+
let path = String::deserialize(deserializer)?;
54+
Ok(ContextFilePath::Agent(path))
55+
}
56+
}
57+
58+
impl ContextFilePath {
59+
pub fn get_path_as_str(&self) -> &str {
60+
match self {
61+
Self::Agent(path) | Self::Session(path) => path.as_str(),
62+
}
63+
}
64+
}
65+
66+
impl PartialEq for ContextFilePath {
67+
fn eq(&self, other: &Self) -> bool {
68+
let self_path = self.get_path_as_str();
69+
let other_path = other.get_path_as_str();
70+
71+
self_path == other_path
72+
}
73+
}
74+
75+
impl PartialEq<str> for ContextFilePath {
76+
fn eq(&self, other: &str) -> bool {
77+
self.get_path_as_str() == other
78+
}
79+
}
80+
81+
impl PartialEq<ContextFilePath> for String {
82+
fn eq(&self, other: &ContextFilePath) -> bool {
83+
let inner = match other {
84+
ContextFilePath::Agent(path) | ContextFilePath::Session(path) => path,
85+
};
86+
87+
self == inner
88+
}
89+
}
90+
2691
/// Manager for context files and profiles.
2792
#[derive(Debug, Clone, Serialize, Deserialize)]
2893
pub struct ContextManager {
2994
max_context_files_size: usize,
3095
/// Name of the current active profile.
3196
pub current_profile: String,
3297
/// List of file paths or glob patterns to include in the context.
33-
pub paths: Vec<String>,
98+
pub paths: Vec<ContextFilePath>,
3499
/// Map of Hook Name to [`Hook`]. The hook name serves as the hook's ID.
35100
pub hooks: HashMap<HookTrigger, Vec<Hook>>,
36101
#[serde(skip)]
@@ -43,7 +108,7 @@ impl ContextManager {
43108
.resources
44109
.iter()
45110
.filter(|resource| resource.starts_with("file://"))
46-
.map(|s| s.trim_start_matches("file://").to_string())
111+
.map(|s| ContextFilePath::Agent(s.trim_start_matches("file://").to_string()))
47112
.collect::<Vec<_>>();
48113

49114
Ok(Self {
@@ -79,12 +144,14 @@ impl ContextManager {
79144
}
80145
}
81146

82-
// Add each path, checking for duplicates
83147
for path in paths {
84-
if self.paths.contains(&path) {
148+
if self.paths.iter().any(|p| p == path.as_str()) {
85149
return Err(eyre!("Rule '{}' already exists.", path));
86150
}
87-
self.paths.push(path);
151+
152+
// The assumption here is that we are only calling [add_paths] for adding paths in
153+
// session
154+
self.paths.push(ContextFilePath::Session(path));
88155
}
89156

90157
Ok(())
@@ -100,7 +167,8 @@ impl ContextManager {
100167
pub fn remove_paths(&mut self, paths: Vec<String>) -> Result<()> {
101168
// Remove each path if it exists
102169
let old_path_num = self.paths.len();
103-
self.paths.retain(|p| !paths.contains(p));
170+
self.paths
171+
.retain(|p| !paths.iter().any(|path| path.as_str() == p.get_path_as_str()));
104172

105173
if old_path_num == self.paths.len() {
106174
return Err(eyre!("None of the specified paths were found in the context"));
@@ -161,12 +229,12 @@ impl ContextManager {
161229
async fn collect_context_files(
162230
&self,
163231
os: &Os,
164-
paths: &[String],
232+
paths: &[ContextFilePath],
165233
context_files: &mut Vec<(String, String)>,
166234
) -> Result<()> {
167235
for path in paths {
168236
// Use is_validation=false to handle non-matching globs gracefully
169-
process_path(os, path, context_files, false).await?;
237+
process_path(os, path.get_path_as_str(), context_files, false).await?;
170238
}
171239
Ok(())
172240
}

crates/chat-cli/src/cli/chat/skim_integration.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ pub fn select_context_paths_with_skim(context_manager: &ContextManager) -> Resul
165165

166166
// Get profile-specific paths
167167
for path in &context_manager.paths {
168-
all_paths.push(format!("(agent: {}) {}", context_manager.current_profile, path));
168+
all_paths.push(format!(
169+
"(agent: {}) {}",
170+
context_manager.current_profile,
171+
path.get_path_as_str()
172+
));
169173
}
170174

171175
if all_paths.is_empty() {

crates/chat-cli/src/cli/chat/tools/gh_issue.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,15 @@ impl GhIssue {
145145
if os_manager.paths.is_empty() {
146146
os_str.push_str("profile_context=none\n\n");
147147
} else {
148-
os_str.push_str(&format!("profile_context=\n{}\n\n", &os_manager.paths.join("\n")));
148+
os_str.push_str(&format!(
149+
"profile_context=\n{}\n\n",
150+
&os_manager
151+
.paths
152+
.iter()
153+
.map(|p| p.get_path_as_str())
154+
.collect::<Vec<_>>()
155+
.join("\n")
156+
));
149157
}
150158

151159
// Handle context files

0 commit comments

Comments
 (0)