diff --git a/.forge/skills/resolve-fixme/SKILL.md b/.forge/skills/resolve-fixme/SKILL.md index eb694b48f2..4c4d721dd4 100644 --- a/.forge/skills/resolve-fixme/SKILL.md +++ b/.forge/skills/resolve-fixme/SKILL.md @@ -1,6 +1,6 @@ --- name: resolve-fixme -description: Find all FIXME comments across the codebase and attempt to resolve them. Use when the user asks to fix, resolve, or address FIXME comments, or when running the "fixme" command. Runs a script to locate every FIXME with surrounding context (2 lines before, 5 lines after) and then works through each one systematically. +description: Find all FIXME comments across the codebase and fully implement the work they describe. Use when the user asks to fix, resolve, or address FIXME comments, or when running the "fixme" command. Runs a discovery script to find every FIXME, expands multiline comment blocks, groups related FIXMEs across files into a single implementation task, completes the full underlying code changes, removes the FIXME comments only after the work is done, and verifies that no FIXMEs remain. --- # Resolve FIXME Comments @@ -20,34 +20,91 @@ bash .forge/skills/resolve-fixme/scripts/find-fixme.sh [PATH] - Skips `.git/`, `target/`, `node_modules/`, and `vendor/`. - Requires either `rg` (ripgrep) or `grep` + `python3`. -### 2. Triage the results +### 2. Expand each FIXME into its full instruction -Read the script output and build a work list. For each FIXME note: -- The file and line number (shown in the header of each block). -- The surrounding context to understand what the FIXME is asking for. -- Whether the fix requires code changes, further research, or is blocked. +Do not rely on the discovery output alone. -### 3. Resolve each FIXME +For every hit: -Work through the list one at a time: +1. Open the file and read around the reported line. +2. Expand the FIXME to include the **entire comment block**. +3. Treat all consecutive related comment lines as part of the same instruction. -1. Read the full file section to understand the intent. -2. Implement the fix — edit the code, add the missing logic, or refactor as needed. -3. Remove the FIXME comment once the issue is resolved. -4. If a FIXME cannot be safely resolved (e.g. requires external input or is intentionally deferred), leave it in place and note why. +Important: -### 4. Verify +- A FIXME may be **multiline**. The line containing `FIXME` is often only the beginning. +- The real instruction may continue on following comment lines and may contain the actual implementation details. +- Do not interpret or edit a FIXME until you have read the full block. -After resolving all FIXMEs, run the project's standard verification steps: +For each expanded FIXME, capture: -``` +- file path +- start line and end line of the full comment block +- a short summary of what that FIXME is asking for + +### 3. Consolidate related FIXMEs across files + +Before editing code, review **all** expanded FIXMEs together. + +Many FIXMEs describe different facets of the same underlying task across multiple files. For example: + +- one file may describe a domain type that needs to be introduced +- another may describe a parameter that should disappear once that type exists +- another may describe a service, repo, or UI update needed to complete the same refactor + +Group such FIXMEs into a single implementation task. + +When grouping, look for: + +- shared vocabulary +- references to the same type, service, repo, parameter, or feature +- comments that clearly describe prerequisite and follow-up changes in different files +- comments that only make sense when read together + +For each group, produce one consolidated understanding of the task: + +- all files and line ranges involved +- the complete implementation required across the group +- the order in which the changes should be made + +Do not resolve grouped FIXMEs one file at a time in isolation. Resolve the whole task consistently. + +### 4. Implement every FIXME completely + +Every FIXME must be resolved. There is no skip path. + +Work through each grouped task until the underlying implementation is complete: + +1. Read any additional files needed to understand the design. +2. Create or modify the required code, types, services, repos, tests, configs, or templates. +3. Propagate the change through every affected file in the group. +4. Remove each FIXME comment **only after** the work it describes has actually been implemented. + +> **Critical rule:** Never delete or rewrite a FIXME comment unless the underlying implementation is finished. The comment is a record of required work. Removing it before completing that work is a failure. + +If the FIXME implies a larger refactor, do the refactor. If it requires creating new supporting code, create it. Do not stop at the first local change if the comment clearly implies additional follow-through elsewhere. + +### 5. Verify + +After resolving all FIXMEs: + +1. Run the project's standard verification step: + +```sh cargo insta test --accept ``` -Re-run the discovery script to confirm no FIXMEs remain unresolved. +2. Re-run the discovery script: + +```sh +bash .forge/skills/resolve-fixme/scripts/find-fixme.sh [PATH] +``` + +3. Confirm that no FIXME comments remain in the targeted scope. ## Notes -- Prefer targeted, minimal fixes — only change what the FIXME describes. -- If the FIXME comment describes a TODO that was intentionally deferred (e.g. `FIXME(later):` or `FIXME(blocked):`), skip it and report it to the user. -- When the context is ambiguous, read more of the surrounding file before making a change. +- Prefer targeted fixes, but do not under-scope the work when multiple FIXMEs describe one larger task. +- Read broadly before editing when the intent is ambiguous. +- Consistency matters more than locality: grouped FIXMEs should lead to one coherent implementation. +- The job is not to clean up comments. The job is to complete the implementation those comments are pointing at. diff --git a/.gitignore b/.gitignore index 077bfbece7..73d39e49e2 100644 --- a/.gitignore +++ b/.gitignore @@ -48,3 +48,4 @@ Cargo.lock **/.forge/request.body.json node_modules/ bench/__pycache__ +.ai/ diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index 8569b21d6b..1372a314ec 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -311,7 +311,6 @@ impl< async fn get_skills(&self) -> Result> { self.infra.load_skills().await } - async fn generate_command(&self, prompt: UserPrompt) -> Result { use forge_app::CommandGenerator; let generator = CommandGenerator::new(self.services.clone()); diff --git a/crates/forge_app/src/command_generator.rs b/crates/forge_app/src/command_generator.rs index 688d3b6f65..78ac004569 100644 --- a/crates/forge_app/src/command_generator.rs +++ b/crates/forge_app/src/command_generator.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use crate::{ AppConfigService, EnvironmentInfra, FileDiscoveryService, ProviderService, TemplateEngine, + TerminalContextService, }; /// Response struct for shell command generation using JSON format @@ -25,14 +26,23 @@ pub struct CommandGenerator { impl CommandGenerator where - S: EnvironmentInfra + FileDiscoveryService + ProviderService + AppConfigService, + S: EnvironmentInfra + + FileDiscoveryService + + ProviderService + + AppConfigService, { /// Creates a new CommandGenerator instance with the provided services. pub fn new(services: Arc) -> Self { Self { services } } - /// Generates a shell command from a natural language prompt + /// Generates a shell command from a natural language prompt. + /// + /// Terminal context is read automatically from the `_FORGE_TERM_COMMANDS`, + /// `_FORGE_TERM_EXIT_CODES`, and `_FORGE_TERM_TIMESTAMPS` environment + /// variables exported by the zsh plugin, and included in the user + /// prompt so the LLM can reference recent commands, exit codes, and + /// timestamps. pub async fn generate(&self, prompt: UserPrompt) -> Result { // Get system information for context let env = self.services.get_environment(); @@ -59,8 +69,22 @@ where } }; - // Build user prompt with task and recent commands - let user_content = format!("{}", prompt.as_str()); + // Build user prompt with task, optionally including terminal context. + use forge_template::Element; + let task_elm = Element::new("task").text(prompt.as_str()); + let terminal_service = TerminalContextService::new(self.services.clone()); + let user_content = match terminal_service.get_terminal_context() { + Some(ctx) => { + let terminal_elm = + Element::new("command_trace").append(ctx.commands.iter().map(|cmd| { + Element::new("command") + .attr("exit_code", cmd.exit_code.to_string()) + .text(&cmd.command) + })); + format!("{}\n\n{}", terminal_elm.render(), task_elm.render()) + } + None => task_elm.render(), + }; // Create context with system and user prompts let ctx = self.create_context(rendered_system_prompt, user_content, &model); @@ -103,7 +127,7 @@ where mod tests { use forge_domain::{ AuthCredential, AuthDetails, AuthMethod, ChatCompletionMessage, Content, FinishReason, - ModelSource, ProviderId, ProviderResponse, ResultStream, + ModelSource, ProviderId, ProviderResponse, ResultStream, Role, }; use tokio::sync::Mutex; use url::Url; @@ -116,6 +140,7 @@ mod tests { response: Arc>>, captured_context: Arc>>, environment: Environment, + env_vars: std::collections::BTreeMap, } impl MockServices { @@ -133,6 +158,26 @@ mod tests { response: Arc::new(Mutex::new(Some(response.to_string()))), captured_context: Arc::new(Mutex::new(None)), environment: env, + env_vars: std::collections::BTreeMap::new(), + }) + } + + fn with_terminal_context( + self: Arc, + commands: &str, + exit_codes: &str, + timestamps: &str, + ) -> Arc { + let mut env_vars = self.env_vars.clone(); + env_vars.insert("_FORGE_TERM_COMMANDS".to_string(), commands.to_string()); + env_vars.insert("_FORGE_TERM_EXIT_CODES".to_string(), exit_codes.to_string()); + env_vars.insert("_FORGE_TERM_TIMESTAMPS".to_string(), timestamps.to_string()); + Arc::new(Self { + files: self.files.clone(), + response: self.response.clone(), + captured_context: self.captured_context.clone(), + environment: self.environment.clone(), + env_vars, }) } } @@ -155,12 +200,12 @@ mod tests { unimplemented!() } - fn get_env_var(&self, _key: &str) -> Option { - None + fn get_env_var(&self, key: &str) -> Option { + self.env_vars.get(key).cloned() } fn get_env_vars(&self) -> std::collections::BTreeMap { - std::collections::BTreeMap::new() + self.env_vars.clone() } } @@ -312,6 +357,35 @@ mod tests { insta::assert_yaml_snapshot!(captured_context); } + #[tokio::test] + async fn test_generate_with_shell_context() { + let fixture = MockServices::new( + r#"{"command": "cargo build --release"}"#, + vec![("Cargo.toml", false)], + ) + .with_terminal_context("cargo build", "101", "1700000000"); + let generator = CommandGenerator::new(fixture.clone()); + + let actual = generator + .generate(UserPrompt::from("fix the command I just ran".to_string())) + .await + .unwrap(); + + assert_eq!(actual, "cargo build --release"); + let captured_context = fixture.captured_context.lock().await.clone().unwrap(); + let user_content = captured_context + .messages + .iter() + .find(|m| m.has_role(Role::User)) + .expect("should have a user message") + .content() + .expect("user message should have content"); + assert!(user_content.contains("")); + assert!(user_content.contains("")); + assert!(user_content.contains("cargo build")); + assert!(user_content.contains("fix the command I just ran")); + } + #[tokio::test] async fn test_generate_fails_when_missing_tag() { let fixture = MockServices::new(r#"{"invalid": "json"}"#, vec![]); diff --git a/crates/forge_app/src/lib.rs b/crates/forge_app/src/lib.rs index 1b3295498c..66de3e618d 100644 --- a/crates/forge_app/src/lib.rs +++ b/crates/forge_app/src/lib.rs @@ -26,6 +26,7 @@ mod services; mod set_conversation_id; pub mod system_prompt; mod template_engine; +mod terminal_context; mod title_generator; mod tool_executor; mod tool_registry; @@ -48,6 +49,7 @@ pub use git_app::*; pub use infra::*; pub use services::*; pub use template_engine::*; +pub use terminal_context::*; pub use tool_resolver::*; pub use user::*; pub use utils::{compute_hash, is_binary_content_type}; diff --git a/crates/forge_app/src/terminal_context.rs b/crates/forge_app/src/terminal_context.rs new file mode 100644 index 0000000000..0079342938 --- /dev/null +++ b/crates/forge_app/src/terminal_context.rs @@ -0,0 +1,317 @@ +use std::sync::Arc; + +use forge_domain::{TerminalCommand, TerminalContext}; + +use crate::EnvironmentInfra; + +/// Environment variable exported by the zsh plugin containing +/// `\x1F`-separated (ASCII Unit Separator) command strings. +pub const ENV_TERM_COMMANDS: &str = "_FORGE_TERM_COMMANDS"; + +/// Environment variable exported by the zsh plugin containing +/// `\x1F`-separated exit codes corresponding to [`ENV_TERM_COMMANDS`]. +pub const ENV_TERM_EXIT_CODES: &str = "_FORGE_TERM_EXIT_CODES"; + +/// Environment variable exported by the zsh plugin containing +/// `\x1F`-separated Unix timestamps corresponding to [`ENV_TERM_COMMANDS`]. +pub const ENV_TERM_TIMESTAMPS: &str = "_FORGE_TERM_TIMESTAMPS"; + +/// The separator used to join and split environment variable lists. +/// +/// ASCII Unit Separator (`\x1F`) is chosen because it cannot appear in +/// shell command strings, paths, URLs, or exit codes — unlike `:` which +/// is common in all of those. +pub const ENV_LIST_SEPARATOR: char = '\x1F'; + +/// Service that reads terminal context from environment variables exported by +/// the zsh plugin and constructs a structured [`TerminalContext`]. +/// +/// The zsh plugin exports three `\x1F`-separated environment variables before +/// invoking forge: +/// - [`ENV_TERM_COMMANDS`] — the command strings +/// - [`ENV_TERM_EXIT_CODES`] — the corresponding exit codes +/// - [`ENV_TERM_TIMESTAMPS`] — the corresponding Unix timestamps +#[derive(Clone)] +pub struct TerminalContextService(Arc); + +impl TerminalContextService { + /// Creates a new `TerminalContextService` backed by the provided + /// infrastructure. + pub fn new(infra: Arc) -> Self { + Self(infra) + } +} + +impl> TerminalContextService { + /// Reads the terminal context from environment variables. + /// + /// Commands are sorted by timestamp (oldest first, most recent last). + /// + /// Returns `None` if none of the required variables are set or if no + /// commands were recorded. + pub fn get_terminal_context(&self) -> Option { + let commands_raw = self.0.get_env_var(ENV_TERM_COMMANDS)?; + + let commands: Vec = split_env_list(&commands_raw); + if commands.is_empty() { + return None; + } + + let exit_codes_raw = self.0.get_env_var(ENV_TERM_EXIT_CODES).unwrap_or_default(); + let timestamps_raw = self.0.get_env_var(ENV_TERM_TIMESTAMPS).unwrap_or_default(); + + let exit_codes: Vec = split_env_list(&exit_codes_raw) + .iter() + .map(|s| s.parse::().unwrap_or(0)) + .collect(); + + let timestamps: Vec = split_env_list(×tamps_raw) + .iter() + .map(|s| s.parse::().unwrap_or(0)) + .collect(); + // Zip the three lists together; pad missing exit codes/timestamps with 0. + // The outer zip() truncates to the length of `commands`, so the + // repeat() padding never produces extra entries. + let mut entries: Vec = commands + .into_iter() + .zip(exit_codes.into_iter().chain(std::iter::repeat(0))) + .zip(timestamps.into_iter().chain(std::iter::repeat(0))) + .map(|((command, exit_code), timestamp)| TerminalCommand { + command, + exit_code, + timestamp, + }) + .collect(); + + // Sort by timestamp so the most recent command appears last. + entries.sort_by_key(|e| e.timestamp); + + if entries.is_empty() { + None + } else { + Some(TerminalContext { commands: entries }) + } + } +} + +/// Splits an `\x1F`-separated (ASCII Unit Separator) environment variable +/// value into a list of strings, filtering out any empty segments. +pub fn split_env_list(raw: &str) -> Vec { + raw.split(ENV_LIST_SEPARATOR) + .filter(|s| !s.is_empty()) + .map(String::from) + .collect() +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + use std::sync::Arc; + + use forge_domain::{Environment, TerminalCommand, TerminalContext}; + use pretty_assertions::assert_eq; + + use super::*; + + struct MockInfra { + env_vars: BTreeMap, + } + + impl MockInfra { + fn new(vars: &[(&str, &str)]) -> Arc { + Arc::new(Self { + env_vars: vars + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + }) + } + } + + impl crate::EnvironmentInfra for MockInfra { + type Config = forge_config::ForgeConfig; + + fn get_environment(&self) -> Environment { + use fake::{Fake, Faker}; + Faker.fake() + } + + fn get_config(&self) -> anyhow::Result { + Ok(forge_config::ForgeConfig::default()) + } + + async fn update_environment( + &self, + _ops: Vec, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn get_env_var(&self, key: &str) -> Option { + self.env_vars.get(key).cloned() + } + + fn get_env_vars(&self) -> BTreeMap { + self.env_vars.clone() + } + } + + #[test] + fn test_no_env_vars_returns_none() { + let fixture = TerminalContextService::new(MockInfra::new(&[])); + let actual = fixture.get_terminal_context(); + assert_eq!(actual, None); + } + + #[test] + fn test_empty_commands_returns_none() { + let fixture = TerminalContextService::new(MockInfra::new(&[(ENV_TERM_COMMANDS, "")])); + let actual = fixture.get_terminal_context(); + assert_eq!(actual, None); + } + + #[test] + fn test_single_command_no_extras() { + let fixture = + TerminalContextService::new(MockInfra::new(&[(ENV_TERM_COMMANDS, "cargo build")])); + let actual = fixture.get_terminal_context(); + let expected = Some(TerminalContext { + commands: vec![TerminalCommand { + command: "cargo build".to_string(), + exit_code: 0, + timestamp: 0, + }], + }); + assert_eq!(actual, expected); + } + + #[test] + fn test_multiple_commands_with_exit_codes_and_timestamps() { + let sep = ENV_LIST_SEPARATOR; + let fixture = TerminalContextService::new(MockInfra::new(&[ + ( + ENV_TERM_COMMANDS, + &format!("ls{sep}cargo test{sep}git status"), + ), + (ENV_TERM_EXIT_CODES, &format!("0{sep}1{sep}0")), + ( + ENV_TERM_TIMESTAMPS, + &format!("1700000001{sep}1700000002{sep}1700000003"), + ), + ])); + let actual = fixture.get_terminal_context(); + let expected = Some(TerminalContext { + commands: vec![ + TerminalCommand { + command: "ls".to_string(), + exit_code: 0, + timestamp: 1700000001, + }, + TerminalCommand { + command: "cargo test".to_string(), + exit_code: 1, + timestamp: 1700000002, + }, + TerminalCommand { + command: "git status".to_string(), + exit_code: 0, + timestamp: 1700000003, + }, + ], + }); + assert_eq!(actual, expected); + } + + #[test] + fn test_split_env_list_empty() { + let actual = split_env_list(""); + let expected: Vec = vec![]; + assert_eq!(actual, expected); + } + + #[test] + fn test_split_env_list_single() { + let actual = split_env_list("hello"); + let expected = vec!["hello".to_string()]; + assert_eq!(actual, expected); + } + + #[test] + fn test_split_env_list_multiple() { + let sep = ENV_LIST_SEPARATOR; + let actual = split_env_list(&format!("a{sep}b{sep}c")); + let expected = vec!["a".to_string(), "b".to_string(), "c".to_string()]; + assert_eq!(actual, expected); + } + + #[test] + fn test_split_env_list_command_with_colon() { + // Commands containing `:` (e.g. URLs, port mappings) must not be split. + let sep = ENV_LIST_SEPARATOR; + let actual = split_env_list(&format!( + "curl https://example.com{sep}docker run -p 8080:80 nginx" + )); + let expected = vec![ + "curl https://example.com".to_string(), + "docker run -p 8080:80 nginx".to_string(), + ]; + assert_eq!(actual, expected); + } + + #[test] + fn test_commands_sorted_by_timestamp_oldest_first() { + // Supply commands in reverse-timestamp order to confirm sorting is applied. + let sep = ENV_LIST_SEPARATOR; + let fixture = TerminalContextService::new(MockInfra::new(&[ + ( + ENV_TERM_COMMANDS, + &format!("git status{sep}cargo test{sep}ls"), + ), + (ENV_TERM_EXIT_CODES, &format!("0{sep}1{sep}0")), + ( + ENV_TERM_TIMESTAMPS, + &format!("1700000003{sep}1700000002{sep}1700000001"), + ), + ])); + let actual = fixture.get_terminal_context(); + let expected = Some(TerminalContext { + commands: vec![ + TerminalCommand { + command: "ls".to_string(), + exit_code: 0, + timestamp: 1700000001, + }, + TerminalCommand { + command: "cargo test".to_string(), + exit_code: 1, + timestamp: 1700000002, + }, + TerminalCommand { + command: "git status".to_string(), + exit_code: 0, + timestamp: 1700000003, + }, + ], + }); + assert_eq!(actual, expected); + } + + #[test] + fn test_all_commands_included() { + // All captured commands are included (no limit). + let sep = ENV_LIST_SEPARATOR; + let fixture = TerminalContextService::new(MockInfra::new(&[ + ( + ENV_TERM_COMMANDS, + &format!("ls{sep}cargo test{sep}git status"), + ), + (ENV_TERM_EXIT_CODES, &format!("0{sep}1{sep}0")), + ( + ENV_TERM_TIMESTAMPS, + &format!("1700000001{sep}1700000002{sep}1700000003"), + ), + ])); + let actual = fixture.get_terminal_context(); + assert_eq!(actual.unwrap().commands.len(), 3); + } +} diff --git a/crates/forge_app/src/user_prompt.rs b/crates/forge_app/src/user_prompt.rs index 382ba8e765..b076c58933 100644 --- a/crates/forge_app/src/user_prompt.rs +++ b/crates/forge_app/src/user_prompt.rs @@ -5,7 +5,7 @@ use forge_domain::{Agent, *}; use serde_json::json; use tracing::debug; -use crate::{AttachmentService, TemplateEngine}; +use crate::{AttachmentService, EnvironmentInfra, TemplateEngine, TerminalContextService}; /// Service responsible for setting user prompts in the conversation context #[derive(Clone)] @@ -16,7 +16,9 @@ pub struct UserPromptGenerator { current_time: chrono::DateTime, } -impl UserPromptGenerator { +impl> + UserPromptGenerator +{ /// Creates a new UserPromptService pub fn new( service: Arc, @@ -52,6 +54,7 @@ impl UserPromptGenerator { } else { conversation }; + Ok(conversation) } @@ -140,53 +143,61 @@ impl UserPromptGenerator { let event_value = self.event.value.clone(); let template_engine = TemplateEngine::default(); - let content = - if let Some(user_prompt) = &self.agent.user_prompt - && self.event.value.is_some() - { - let user_input = self - .event - .value - .as_ref() - .and_then(|v| v.as_user_prompt().map(|u| u.as_str().to_string())) - .unwrap_or_default(); - let mut event_context = EventContext::new(EventContextValue::new(user_input)) - .current_date(self.current_time.format("%Y-%m-%d").to_string()); - - // Check if context already contains user messages to determine if it's feedback - let has_user_messages = context.messages.iter().any(|msg| msg.has_role(Role::User)); - - if has_user_messages { - event_context = event_context.into_feedback(); - } else { - event_context = event_context.into_task(); + let content = if let Some(user_prompt) = &self.agent.user_prompt + && self.event.value.is_some() + { + let user_input = self + .event + .value + .as_ref() + .and_then(|v| v.as_user_prompt().map(|u| u.as_str().to_string())) + .unwrap_or_default(); + let mut event_context = EventContext::new(EventContextValue::new(user_input)) + .current_date(self.current_time.format("%Y-%m-%d").to_string()); + + // Check if context already contains user messages to determine if it's feedback + let has_user_messages = context.messages.iter().any(|msg| msg.has_role(Role::User)); + + if has_user_messages { + event_context = event_context.into_feedback(); + } else { + event_context = event_context.into_task(); + } + + debug!(event_context = ?event_context, "Event context"); + + // Render the command first. + let event_context = match self.event.value.as_ref().and_then(|v| v.as_command()) { + Some(command) => { + let rendered_prompt = template_engine.render_template( + command.template.clone(), + &json!({"parameters": command.parameters.join(" ")}), + )?; + event_context.event(EventContextValue::new(rendered_prompt)) } + None => event_context, + }; - debug!(event_context = ?event_context, "Event context"); - - // Render the command first. - let event_context = match self.event.value.as_ref().and_then(|v| v.as_command()) { - Some(command) => { - let rendered_prompt = template_engine.render_template( - command.template.clone(), - &json!({"parameters": command.parameters.join(" ")}), - )?; - event_context.event(EventContextValue::new(rendered_prompt)) - } + // Inject terminal context into the event context when available. + let event_context = + match TerminalContextService::new(self.services.clone()).get_terminal_context() { + Some(ctx) => event_context.terminal_context(Some(ctx)), None => event_context, }; - // Render the event value into agent's user prompt template. - Some(template_engine.render_template( + // Render the event value into agent's user prompt template. + Some( + template_engine.render_template( Template::new(user_prompt.template.as_str()), &event_context, - )?) - } else { - // Use the raw event value as content if no user_prompt is provided - event_value - .as_ref() - .and_then(|v| v.as_user_prompt().map(|p| p.deref().to_owned())) - }; + )?, + ) + } else { + // Use the raw event value as content if no user_prompt is provided + event_value + .as_ref() + .and_then(|v| v.as_user_prompt().map(|p| p.deref().to_owned())) + }; if let Some(content) = &content { // Create User Message @@ -261,6 +272,34 @@ mod tests { } } + impl crate::EnvironmentInfra for MockService { + type Config = forge_config::ForgeConfig; + + fn get_environment(&self) -> forge_domain::Environment { + use fake::{Fake, Faker}; + Faker.fake() + } + + fn get_config(&self) -> anyhow::Result { + Ok(forge_config::ForgeConfig::default()) + } + + async fn update_environment( + &self, + _ops: Vec, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn get_env_var(&self, _key: &str) -> Option { + None + } + + fn get_env_vars(&self) -> std::collections::BTreeMap { + Default::default() + } + } + fn fixture_agent_without_user_prompt() -> Agent { Agent::new( AgentId::from("test_agent"), @@ -387,6 +426,29 @@ mod tests { // Setup - Create a service that returns file attachments struct MockServiceWithFiles; + impl crate::EnvironmentInfra for MockServiceWithFiles { + type Config = forge_config::ForgeConfig; + fn get_environment(&self) -> forge_domain::Environment { + use fake::{Fake, Faker}; + Faker.fake() + } + fn get_config(&self) -> anyhow::Result { + Ok(forge_config::ForgeConfig::default()) + } + async fn update_environment( + &self, + _ops: Vec, + ) -> anyhow::Result<()> { + Ok(()) + } + fn get_env_var(&self, _key: &str) -> Option { + None + } + fn get_env_vars(&self) -> std::collections::BTreeMap { + Default::default() + } + } + #[async_trait::async_trait] impl AttachmentService for MockServiceWithFiles { async fn attachments(&self, _url: &str) -> anyhow::Result> { @@ -468,6 +530,29 @@ mod tests { // Setup - Simple mock that returns no attachments struct MockServiceWithTodos; + impl crate::EnvironmentInfra for MockServiceWithTodos { + type Config = forge_config::ForgeConfig; + fn get_environment(&self) -> forge_domain::Environment { + use fake::{Fake, Faker}; + Faker.fake() + } + fn get_config(&self) -> anyhow::Result { + Ok(forge_config::ForgeConfig::default()) + } + async fn update_environment( + &self, + _ops: Vec, + ) -> anyhow::Result<()> { + Ok(()) + } + fn get_env_var(&self, _key: &str) -> Option { + None + } + fn get_env_vars(&self) -> std::collections::BTreeMap { + Default::default() + } + } + #[async_trait::async_trait] impl AttachmentService for MockServiceWithTodos { async fn attachments(&self, _url: &str) -> anyhow::Result> { @@ -545,6 +630,29 @@ mod tests { // Setup - Simple mock with no attachments struct MockServiceNoTodos; + impl crate::EnvironmentInfra for MockServiceNoTodos { + type Config = forge_config::ForgeConfig; + fn get_environment(&self) -> forge_domain::Environment { + use fake::{Fake, Faker}; + Faker.fake() + } + fn get_config(&self) -> anyhow::Result { + Ok(forge_config::ForgeConfig::default()) + } + async fn update_environment( + &self, + _ops: Vec, + ) -> anyhow::Result<()> { + Ok(()) + } + fn get_env_var(&self, _key: &str) -> Option { + None + } + fn get_env_vars(&self) -> std::collections::BTreeMap { + Default::default() + } + } + #[async_trait::async_trait] impl AttachmentService for MockServiceNoTodos { async fn attachments(&self, _url: &str) -> anyhow::Result> { diff --git a/crates/forge_config/.forge.toml b/crates/forge_config/.forge.toml index 1abdbc915c..45a08e9231 100644 --- a/crates/forge_config/.forge.toml +++ b/crates/forge_config/.forge.toml @@ -22,6 +22,7 @@ model_cache_ttl_secs = 604800 restricted = false sem_search_top_k = 10 services_url = "https://api.forgecode.dev/" +terminal_context = false tool_supported = true tool_timeout_secs = 300 top_k = 30 diff --git a/crates/forge_domain/src/event.rs b/crates/forge_domain/src/event.rs index 45256e51b2..4609aaf76d 100644 --- a/crates/forge_domain/src/event.rs +++ b/crates/forge_domain/src/event.rs @@ -5,7 +5,7 @@ use derive_setters::Setters; use serde::{Deserialize, Serialize}; use serde_json::Value; -use crate::{Attachment, NamedTool, Template, ToolName}; +use crate::{Attachment, NamedTool, Template, TerminalContext, ToolName}; /// Represents a partial event structure used for CLI event dispatching /// @@ -90,6 +90,10 @@ pub struct EventContext { suggestions: Vec, variables: HashMap, current_date: String, + /// Structured terminal context injected by [`TerminalContextService`], + /// or `None` when terminal context is unavailable or disabled. + #[serde(default, skip_serializing_if = "Option::is_none")] + terminal_context: Option, } #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Setters)] @@ -111,6 +115,7 @@ impl EventContext { suggestions: Default::default(), variables: Default::default(), current_date: chrono::Local::now().format("%Y-%m-%d").to_string(), + terminal_context: None, } } diff --git a/crates/forge_domain/src/lib.rs b/crates/forge_domain/src/lib.rs index 5db0a8553b..5ae3fca85d 100644 --- a/crates/forge_domain/src/lib.rs +++ b/crates/forge_domain/src/lib.rs @@ -43,6 +43,7 @@ mod suggestion; mod system_context; mod temperature; mod template; +mod terminal_context; mod tools; mod tool_order; @@ -98,6 +99,7 @@ pub use suggestion::*; pub use system_context::*; pub use temperature::*; pub use template::*; +pub use terminal_context::*; pub use tool_order::*; pub use tools::*; pub use top_k::*; diff --git a/crates/forge_domain/src/terminal_context.rs b/crates/forge_domain/src/terminal_context.rs new file mode 100644 index 0000000000..0405be2565 --- /dev/null +++ b/crates/forge_domain/src/terminal_context.rs @@ -0,0 +1,49 @@ +/// A single command entry captured by the shell plugin. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct TerminalCommand { + /// The command text as entered by the user. + pub command: String, + /// The exit code produced by the command. + pub exit_code: i32, + /// Unix timestamp (seconds since epoch) when the command was run. + pub timestamp: u64, +} + +/// Structured terminal context captured by the shell plugin. +/// +/// Each field corresponds to one of the environment variables exported by the +/// zsh plugin before invoking forge: +/// - `_FORGE_TERM_COMMANDS` — `\x1F`-separated command strings +/// - `_FORGE_TERM_EXIT_CODES` — `\x1F`-separated exit codes +/// - `_FORGE_TERM_TIMESTAMPS` — `\x1F`-separated Unix timestamps +#[derive(Debug, Clone, PartialEq, Eq, Default, serde::Serialize, serde::Deserialize)] +pub struct TerminalContext { + /// Ordered list of recent commands, from oldest to newest. + pub commands: Vec, +} + +impl TerminalContext { + /// Creates a new `TerminalContext` from parallel vectors of command data. + /// + /// All three slices must have the same length; entries at the same index + /// are combined into a single [`TerminalCommand`]. If the lengths differ, + /// the shortest slice determines how many entries are produced. + pub fn new(commands: Vec, exit_codes: Vec, timestamps: Vec) -> Self { + let entries = commands + .into_iter() + .zip(exit_codes) + .zip(timestamps) + .map(|((command, exit_code), timestamp)| TerminalCommand { + command, + exit_code, + timestamp, + }) + .collect(); + Self { commands: entries } + } + + /// Returns `true` if there are no recorded commands. + pub fn is_empty(&self) -> bool { + self.commands.is_empty() + } +} diff --git a/crates/forge_repo/src/agents/forge.md b/crates/forge_repo/src/agents/forge.md index 1bb59d4b4e..14fea74699 100644 --- a/crates/forge_repo/src/agents/forge.md +++ b/crates/forge_repo/src/agents/forge.md @@ -23,6 +23,13 @@ tools: user_prompt: |- <{{event.name}}>{{event.value}} {{current_date}} + {{#if terminal_context}} + + {{#each terminal_context.commands}} + {{command}} + {{/each}} + + {{/if}} --- You are Forge, an expert software engineering assistant designed to help users with programming tasks, file operations, and software development processes. Your knowledge spans multiple programming languages, frameworks, design patterns, and best practices. diff --git a/crates/forge_repo/src/agents/muse.md b/crates/forge_repo/src/agents/muse.md index d990f9a43d..39f341647c 100644 --- a/crates/forge_repo/src/agents/muse.md +++ b/crates/forge_repo/src/agents/muse.md @@ -15,6 +15,13 @@ tools: user_prompt: |- <{{event.name}}>{{event.value}} {{current_date}} + {{#if terminal_context}} + + {{#each terminal_context.commands}} + {{command}} + {{/each}} + + {{/if}} --- You are Muse, an expert strategic planning and analysis assistant designed to help users with detailed implementation planning. Your primary function is to analyze requirements, create structured plans, and provide strategic recommendations without making any actual changes to the codebase or repository. diff --git a/crates/forge_repo/src/agents/sage.md b/crates/forge_repo/src/agents/sage.md index 3e314f7044..0287a22101 100644 --- a/crates/forge_repo/src/agents/sage.md +++ b/crates/forge_repo/src/agents/sage.md @@ -12,6 +12,13 @@ tools: user_prompt: |- <{{event.name}}>{{event.value}} {{current_date}} + {{#if terminal_context}} + + {{#each terminal_context.commands}} + {{command}} + {{/each}} + + {{/if}} --- You are Sage, an expert codebase research and exploration assistant designed to help users understand software projects through deep analysis and investigation. Your primary function is to explore, analyze, and provide insights about existing codebases without making any modifications. diff --git a/shell-plugin/forge.plugin.zsh b/shell-plugin/forge.plugin.zsh index e877afdff7..45137add7a 100755 --- a/shell-plugin/forge.plugin.zsh +++ b/shell-plugin/forge.plugin.zsh @@ -13,6 +13,9 @@ source "${0:A:h}/lib/highlight.zsh" # Core utilities (includes logging) source "${0:A:h}/lib/helpers.zsh" +# Terminal context capture (preexec/precmd hooks, OSC 133) +source "${0:A:h}/lib/context.zsh" + # Completion widget source "${0:A:h}/lib/completion.zsh" diff --git a/shell-plugin/lib/actions/editor.zsh b/shell-plugin/lib/actions/editor.zsh index e03bf79d16..2fcaef52d0 100644 --- a/shell-plugin/lib/actions/editor.zsh +++ b/shell-plugin/lib/actions/editor.zsh @@ -80,10 +80,11 @@ function _forge_action_suggest() { fi echo + # Generate the command local generated_command generated_command=$(FORCE_COLOR=true CLICOLOR_FORCE=1 _forge_exec suggest "$description") - + if [[ -n "$generated_command" ]]; then # Replace the buffer with the generated command BUFFER="$generated_command" diff --git a/shell-plugin/lib/config.zsh b/shell-plugin/lib/config.zsh index bbc05371e2..4086f72f27 100644 --- a/shell-plugin/lib/config.zsh +++ b/shell-plugin/lib/config.zsh @@ -38,3 +38,15 @@ typeset -h _FORGE_SESSION_PROVIDER # Session-scoped reasoning effort override (set via :reasoning-effort / :re). # When non-empty, exported as FORGE_REASONING__EFFORT for every forge invocation. typeset -h _FORGE_SESSION_REASONING_EFFORT + +# Terminal context capture settings +# Master switch for terminal context capture (preexec/precmd hooks) +typeset -h _FORGE_TERM_ENABLED="${FORGE_TERM_ENABLED:-true}" +# Maximum number of commands to keep in the ring buffer (metadata: cmd + exit code) +typeset -h _FORGE_TERM_MAX_COMMANDS="${FORGE_TERM_MAX_COMMANDS:-5}" +# OSC 133 semantic prompt marker emission: "auto", "on", or "off" +typeset -h _FORGE_TERM_OSC133="${FORGE_TERM_OSC133:-auto}" +# Ring buffer arrays for context capture +typeset -ha _FORGE_TERM_COMMANDS=() +typeset -ha _FORGE_TERM_EXIT_CODES=() +typeset -ha _FORGE_TERM_TIMESTAMPS=() diff --git a/shell-plugin/lib/context.zsh b/shell-plugin/lib/context.zsh new file mode 100644 index 0000000000..207a58b487 --- /dev/null +++ b/shell-plugin/lib/context.zsh @@ -0,0 +1,121 @@ +#!/usr/bin/env zsh + +# Terminal context capture for forge plugin +# +# Provides three layers of terminal context: +# 1. preexec/precmd hooks: ring buffer of recent commands + exit codes +# 2. OSC 133 emission: semantic terminal markers for compatible terminals +# 3. Terminal-specific output capture: Kitty > WezTerm > tmux +# +# Context is organized by command blocks: each command's metadata and its +# full output are grouped together, using the known command strings from +# the ring buffer to detect boundaries in the terminal scrollback. + +# --------------------------------------------------------------------------- +# OSC 133 helpers +# --------------------------------------------------------------------------- + +# Determines whether OSC 133 semantic markers should be emitted. +# Auto-detection is conservative: only emit for terminals known to support it +# to avoid garbled output in unsupported terminals. +# The detection result is cached per session in _FORGE_TERM_OSC133_CACHED +# ("1" = emit, "0" = don't emit) to avoid repeated detection overhead. +typeset -g _FORGE_TERM_OSC133_CACHED="" +function _forge_osc133_should_emit() { + if [[ -n "$_FORGE_TERM_OSC133_CACHED" ]]; then + [[ "$_FORGE_TERM_OSC133_CACHED" == "1" ]] && return 0 || return 1 + fi + case "$_FORGE_TERM_OSC133" in + on) _FORGE_TERM_OSC133_CACHED="1"; return 0 ;; + off) _FORGE_TERM_OSC133_CACHED="0"; return 1 ;; + auto) + # Kitty sets KITTY_PID + if [[ -n "${KITTY_PID:-}" ]]; then _FORGE_TERM_OSC133_CACHED="1"; return 0; fi + # Detect by TERM_PROGRAM + case "${TERM_PROGRAM:-}" in + WezTerm|iTerm.app|vscode) _FORGE_TERM_OSC133_CACHED="1"; return 0 ;; + esac + # Foot terminal + if [[ "${TERM:-}" == "foot"* ]]; then _FORGE_TERM_OSC133_CACHED="1"; return 0; fi + # Ghostty + if [[ "${TERM_PROGRAM:-}" == "ghostty" ]]; then _FORGE_TERM_OSC133_CACHED="1"; return 0; fi + # Unknown terminal: don't emit + _FORGE_TERM_OSC133_CACHED="0" + return 1 + ;; + *) _FORGE_TERM_OSC133_CACHED="0"; return 1 ;; + esac +} + +# Emits an OSC 133 marker if the terminal supports it. +# Usage: _forge_osc133_emit "A" or _forge_osc133_emit "D;0" +function _forge_osc133_emit() { + _forge_osc133_should_emit || return 0 + printf '\e]133;%s\a' "$1" +} + +# --------------------------------------------------------------------------- +# preexec / precmd hooks +# --------------------------------------------------------------------------- + +# Ring buffer storage uses parallel arrays declared in config.zsh: +# _FORGE_TERM_COMMANDS, _FORGE_TERM_EXIT_CODES, _FORGE_TERM_TIMESTAMPS +# Pending command state: +typeset -g _FORGE_TERM_PENDING_CMD="" +typeset -g _FORGE_TERM_PENDING_TS="" + +# Called before each command executes. +# Records the command text and timestamp, emits OSC 133 B+C markers. +function _forge_context_preexec() { + [[ "$_FORGE_TERM_ENABLED" != "true" ]] && return + _FORGE_TERM_PENDING_CMD="$1" + _FORGE_TERM_PENDING_TS="$(date +%s)" + # OSC 133 B: prompt end / command start + _forge_osc133_emit "B" + # OSC 133 C: command output start + _forge_osc133_emit "C" +} + +# Called after each command completes, before the next prompt is drawn. +# Captures exit code, pushes to ring buffer, emits OSC 133 D+A markers. +function _forge_context_precmd() { + local last_exit=$? # MUST be first line to capture exit code + + # OSC 133 D: command finished with exit code. + # Emitted unconditionally (before the enabled check) so that terminals + # relying on paired A/B/C/D markers never receive an unpaired sequence, + # even when context capture is disabled. + _forge_osc133_emit "D;$last_exit" + + [[ "$_FORGE_TERM_ENABLED" != "true" ]] && return + + # Only record if we have a pending command from preexec + if [[ -n "$_FORGE_TERM_PENDING_CMD" ]]; then + _FORGE_TERM_COMMANDS+=("$_FORGE_TERM_PENDING_CMD") + _FORGE_TERM_EXIT_CODES+=("$last_exit") + _FORGE_TERM_TIMESTAMPS+=("$_FORGE_TERM_PENDING_TS") + + # Trim ring buffer to max size + while (( ${#_FORGE_TERM_COMMANDS} > _FORGE_TERM_MAX_COMMANDS )); do + shift _FORGE_TERM_COMMANDS + shift _FORGE_TERM_EXIT_CODES + shift _FORGE_TERM_TIMESTAMPS + done + + _FORGE_TERM_PENDING_CMD="" + _FORGE_TERM_PENDING_TS="" + fi + + # OSC 133 A: prompt start (for the next prompt) + _forge_osc133_emit "A" +} + +# Hook registration + +# Register using standard zsh hook arrays for coexistence with other plugins. +# precmd is prepended so it runs first and captures the real $? from the +# command, before other plugins (powerlevel10k, starship, etc.) overwrite it. +if [[ "$_FORGE_TERM_ENABLED" == "true" ]]; then + preexec_functions+=(_forge_context_preexec) + precmd_functions=(_forge_context_precmd "${precmd_functions[@]}") +fi diff --git a/shell-plugin/lib/helpers.zsh b/shell-plugin/lib/helpers.zsh index 36cbb4d59f..a7a62a57e9 100644 --- a/shell-plugin/lib/helpers.zsh +++ b/shell-plugin/lib/helpers.zsh @@ -22,6 +22,25 @@ function _forge_exec() { local agent_id="${_FORGE_ACTIVE_AGENT:-forge}" local -a cmd cmd=($_FORGE_BIN --agent "$agent_id") + + # Expose terminal context arrays as US-separated (\x1F) env vars so that + # the Rust TerminalContextService can read them via get_env_var. + # ASCII Unit Separator (\x1F) is used instead of `:` because commands + # can legitimately contain colons (URLs, port mappings, paths, etc.). + # Use `local -x` so the variables are exported only to the child forge + # process and do not leak into the caller's shell environment. + if [[ "$_FORGE_TERM_ENABLED" == "true" && ${#_FORGE_TERM_COMMANDS} -gt 0 ]]; then + # Join the ring-buffer arrays with the ASCII Unit Separator (\x1F). + # We use IFS-based joining ("${arr[*]}") rather than ${(j.SEP.)arr} because + # zsh does NOT expand $'...' ANSI-C escapes inside parameter expansion flags. + local _old_ifs="$IFS" _sep=$'\x1f' + IFS="$_sep" + local -x _FORGE_TERM_COMMANDS="${_FORGE_TERM_COMMANDS[*]}" + local -x _FORGE_TERM_EXIT_CODES="${_FORGE_TERM_EXIT_CODES[*]}" + local -x _FORGE_TERM_TIMESTAMPS="${_FORGE_TERM_TIMESTAMPS[*]}" + IFS="$_old_ifs" + fi + cmd+=("$@") [[ -n "$_FORGE_SESSION_MODEL" ]] && local -x FORGE_SESSION__MODEL_ID="$_FORGE_SESSION_MODEL" [[ -n "$_FORGE_SESSION_PROVIDER" ]] && local -x FORGE_SESSION__PROVIDER_ID="$_FORGE_SESSION_PROVIDER" @@ -39,6 +58,23 @@ function _forge_exec_interactive() { local agent_id="${_FORGE_ACTIVE_AGENT:-forge}" local -a cmd cmd=($_FORGE_BIN --agent "$agent_id") + + # Expose terminal context arrays as US-separated (\x1F) env vars so that + # the Rust TerminalContextService can read them via get_env_var. + # ASCII Unit Separator (\x1F) is used instead of `:` because commands + # can legitimately contain colons (URLs, port mappings, paths, etc.). + # Use `local -x` so the variables are exported only for the duration of + # this function call (i.e. inherited by the child forge process) and do + # not leak into the caller's shell environment. + if [[ "$_FORGE_TERM_ENABLED" == "true" && ${#_FORGE_TERM_COMMANDS} -gt 0 ]]; then + local _old_ifs="$IFS" _sep=$'\x1f' + IFS="$_sep" + local -x _FORGE_TERM_COMMANDS="${_FORGE_TERM_COMMANDS[*]}" + local -x _FORGE_TERM_EXIT_CODES="${_FORGE_TERM_EXIT_CODES[*]}" + local -x _FORGE_TERM_TIMESTAMPS="${_FORGE_TERM_TIMESTAMPS[*]}" + IFS="$_old_ifs" + fi + cmd+=("$@") [[ -n "$_FORGE_SESSION_MODEL" ]] && local -x FORGE_SESSION__MODEL_ID="$_FORGE_SESSION_MODEL" [[ -n "$_FORGE_SESSION_PROVIDER" ]] && local -x FORGE_SESSION__PROVIDER_ID="$_FORGE_SESSION_PROVIDER"