From f4c4fbf167ce74656e883516380da20bd629417c Mon Sep 17 00:00:00 2001 From: hz-xiaxz <1806656034@qq.com> Date: Sun, 6 Sep 2026 23:50:55 -0400 Subject: [PATCH 1/2] fix(provider): add Gemini 3.8, 3.7, 3.6 Flash and 3.5 Flash-Lite models Add the four Gemini models Google released between July and September 2026 to the vertex_ai and google_ai_studio providers, and to vertex.json: - gemini-3.8-flash - gemini-3.7-flash - gemini-3.6-flash - gemini-3.5-flash-lite All four expose a 1,048,576-token input window and support function calling and thinking, matching the existing gemini-3.5-flash entry. The Flash Cyber variants are intentionally left out because they are only available through Google's limited-access Fairwind program. Co-Authored-By: Claude Fable 5.1 --- crates/forge_repo/src/provider/provider.json | 80 ++++++++++++++++++++ vertex.json | 40 ++++++++++ 2 files changed, 120 insertions(+) diff --git a/crates/forge_repo/src/provider/provider.json b/crates/forge_repo/src/provider/provider.json index 27da81ff9a..8e0cbd0520 100644 --- a/crates/forge_repo/src/provider/provider.json +++ b/crates/forge_repo/src/provider/provider.json @@ -1467,6 +1467,36 @@ "response_type": "Google", "url": "https://{{#if (eq LOCATION \"global\")}}aiplatform.googleapis.com{{else}}{{LOCATION}}-aiplatform.googleapis.com{{/if}}/v1beta1/projects/{{PROJECT_ID}}/locations/{{LOCATION}}/publishers/google", "models": [ + { + "id": "gemini-3.8-flash", + "name": "Gemini 3.8 Flash", + "description": "Google's most intelligent Flash model, optimized for agentic coding and tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "gemini-3.7-flash", + "name": "Gemini 3.7 Flash", + "description": "Google's everyday agentic workhorse with improved instruction following and tool calling", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "gemini-3.6-flash", + "name": "Gemini 3.6 Flash", + "description": "Google's fast, cost-efficient workhorse model with native tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "gemini-3.5-flash", "name": "Gemini 3.5 Flash", @@ -1477,6 +1507,16 @@ "supports_reasoning": true, "input_modalities": ["text", "image"] }, + { + "id": "gemini-3.5-flash-lite", + "name": "Gemini 3.5 Flash-Lite", + "description": "Google's fastest and most cost-efficient Gemini model, optimized for low latency use cases", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "gemini-3.1-flash-lite-preview", "name": "Gemini 3.1 Flash Lite Preview", @@ -2846,6 +2886,36 @@ "response_type": "Google", "url": "https://generativelanguage.googleapis.com/v1beta", "models": [ + { + "id": "gemini-3.8-flash", + "name": "Gemini 3.8 Flash", + "description": "Google's most intelligent Flash model, optimized for agentic coding and tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "gemini-3.7-flash", + "name": "Gemini 3.7 Flash", + "description": "Google's everyday agentic workhorse with improved instruction following and tool calling", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "gemini-3.6-flash", + "name": "Gemini 3.6 Flash", + "description": "Google's fast, cost-efficient workhorse model with native tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "gemini-3.5-flash", "name": "Gemini 3.5 Flash", @@ -2856,6 +2926,16 @@ "supports_reasoning": true, "input_modalities": ["text", "image"] }, + { + "id": "gemini-3.5-flash-lite", + "name": "Gemini 3.5 Flash-Lite", + "description": "Google's fastest and most cost-efficient Gemini model, optimized for low latency use cases", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "gemini-3.1-pro-preview", "name": "Gemini 3.1 Pro Preview", diff --git a/vertex.json b/vertex.json index cf8548a44c..038f9ea7ca 100644 --- a/vertex.json +++ b/vertex.json @@ -1,4 +1,34 @@ [ + { + "id": "google/gemini-3.8-flash", + "name": "Gemini 3.8 Flash", + "description": "Google's most intelligent Flash model, optimized for agentic coding and tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "google/gemini-3.7-flash", + "name": "Gemini 3.7 Flash", + "description": "Google's everyday agentic workhorse with improved instruction following and tool calling", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, + { + "id": "google/gemini-3.6-flash", + "name": "Gemini 3.6 Flash", + "description": "Google's fast, cost-efficient workhorse model with native tool use", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "google/gemini-3.5-flash", "name": "Gemini 3.5 Flash", @@ -9,6 +39,16 @@ "supports_reasoning": true, "input_modalities": ["text", "image"] }, + { + "id": "google/gemini-3.5-flash-lite", + "name": "Gemini 3.5 Flash-Lite", + "description": "Google's fastest and most cost-efficient Gemini model, optimized for low latency use cases", + "context_length": 1048576, + "tools_supported": true, + "supports_parallel_tool_calls": true, + "supports_reasoning": true, + "input_modalities": ["text", "image"] + }, { "id": "mistral/mistral-large-2411", "name": "Mistral Large 2411", From 5bfe035bc2e4f204d87f3f425ff10eaa140a81e4 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 05:18:21 +0000 Subject: [PATCH 2/2] [autofix.ci] apply automated fixes --- crates/forge_api/src/forge_api.rs | 4 +- crates/forge_app/src/agent.rs | 12 +- crates/forge_app/src/agent_executor.rs | 5 +- .../forge_app/src/agent_provider_resolver.rs | 3 +- crates/forge_app/src/app.rs | 10 +- crates/forge_app/src/command_generator.rs | 3 +- crates/forge_app/src/compact.rs | 42 +++--- crates/forge_app/src/dto/anthropic/request.rs | 23 ++-- .../forge_app/src/dto/anthropic/response.rs | 21 +-- .../anthropic/transforms/enforce_schema.rs | 3 +- .../anthropic/transforms/mcp_tool_names.rs | 6 +- .../src/dto/anthropic/transforms/set_cache.rs | 3 +- crates/forge_app/src/dto/google/request.rs | 21 +-- crates/forge_app/src/dto/google/response.rs | 3 +- crates/forge_app/src/dto/openai/error.rs | 3 +- crates/forge_app/src/dto/openai/model.rs | 11 +- crates/forge_app/src/dto/openai/request.rs | 9 +- crates/forge_app/src/dto/openai/response.rs | 17 ++- .../transformers/ensure_system_first.rs | 3 +- .../transformers/github_copilot_reasoning.rs | 3 +- .../openai/transformers/make_openai_compat.rs | 6 +- .../transformers/normalize_tool_schema.rs | 3 +- .../src/dto/openai/transformers/pipeline.rs | 12 +- .../openai/transformers/reasoning_content.rs | 3 +- .../src/dto/openai/transformers/set_cache.rs | 3 +- .../transformers/set_reasoning_effort.rs | 9 +- .../dto/openai/transformers/tool_choice.rs | 4 +- crates/forge_app/src/file_tracking.rs | 6 +- crates/forge_app/src/fmt/fmt_input.rs | 3 +- crates/forge_app/src/fmt/todo_fmt.rs | 8 +- crates/forge_app/src/git_app.rs | 17 ++- crates/forge_app/src/hooks/doom_loop.rs | 23 ++-- crates/forge_app/src/hooks/pending_todos.rs | 7 +- .../forge_app/src/hooks/title_generation.rs | 7 +- crates/forge_app/src/mcp_executor.rs | 5 +- crates/forge_app/src/operation.rs | 13 +- crates/forge_app/src/orch.rs | 46 ++++--- crates/forge_app/src/orch_spec/orch_spec.rs | 10 +- crates/forge_app/src/search_dedup.rs | 3 +- crates/forge_app/src/system_prompt.rs | 22 ++-- crates/forge_app/src/template_engine.rs | 3 +- crates/forge_app/src/terminal_context.rs | 9 +- crates/forge_app/src/title_generator.rs | 7 +- crates/forge_app/src/tool_registry.rs | 26 ++-- crates/forge_app/src/tool_resolver.rs | 8 +- .../transformers/model_specific_reasoning.rs | 11 +- .../src/transformers/strip_working_dir.rs | 11 +- .../src/transformers/trim_context_summary.rs | 9 +- crates/forge_app/src/user_prompt.rs | 23 ++-- crates/forge_app/src/utils.rs | 81 +++++++----- crates/forge_display/src/code.rs | 6 +- crates/forge_display/src/diff.rs | 8 +- crates/forge_display/src/grep.rs | 3 +- crates/forge_domain/src/agent.rs | 40 +++--- .../src/compact/compact_config.rs | 3 +- crates/forge_domain/src/compact/strategy.rs | 38 +++--- crates/forge_domain/src/compact/summary.rs | 3 +- crates/forge_domain/src/context.rs | 62 +++++---- crates/forge_domain/src/message.rs | 3 +- crates/forge_domain/src/policies/engine.rs | 4 +- crates/forge_domain/src/policies/policy.rs | 6 +- crates/forge_domain/src/provider.rs | 3 +- crates/forge_domain/src/reasoning.rs | 7 +- crates/forge_domain/src/result_stream_ext.rs | 99 ++++++++------ crates/forge_domain/src/session_metrics.rs | 3 +- crates/forge_domain/src/tool_order.rs | 4 +- crates/forge_domain/src/tools/call/args.rs | 16 ++- crates/forge_domain/src/tools/call/parser.rs | 3 +- .../forge_domain/src/tools/call/tool_call.rs | 23 ++-- crates/forge_domain/src/tools/catalog.rs | 6 +- .../forge_domain/src/tools/definition/name.rs | 3 +- .../src/tools/definition/tool_definition.rs | 3 +- .../src/transformer/normalize_tool_args.rs | 6 +- .../src/transformer/reasoning_normalizer.rs | 26 ++-- .../forge_domain/src/transformer/set_model.rs | 3 +- .../src/transformer/sort_tools.rs | 3 +- .../src/transformer/transform_tool_calls.rs | 6 +- .../tests/test_stringified_tool_calls.rs | 12 +- crates/forge_fs/src/binary_detection.rs | 17 ++- .../forge_infra/src/auth/mcp_token_storage.rs | 3 +- crates/forge_infra/src/auth/strategy.rs | 31 +++-- crates/forge_infra/src/console.rs | 6 +- crates/forge_infra/src/env.rs | 11 +- crates/forge_infra/src/executor.rs | 13 +- crates/forge_infra/src/kv_storage.rs | 8 +- crates/forge_infra/src/mcp_client.rs | 21 +-- crates/forge_json_repair/src/parser.rs | 15 ++- .../forge_json_repair/src/schema_coercion.rs | 37 +++--- crates/forge_json_repair/tests/error_cases.rs | 41 +++--- crates/forge_main/src/completer/command.rs | 13 +- .../src/completer/input_completer.rs | 4 +- .../forge_main/src/conversation_selector.rs | 4 +- crates/forge_main/src/display_constants.rs | 3 +- crates/forge_main/src/highlighter.rs | 22 ++-- crates/forge_main/src/info.rs | 16 ++- crates/forge_main/src/main.rs | 3 +- crates/forge_main/src/model.rs | 15 ++- crates/forge_main/src/porcelain.rs | 9 +- crates/forge_main/src/sandbox.rs | 4 +- crates/forge_main/src/stream_renderer.rs | 12 +- crates/forge_main/src/ui.rs | 123 +++++++++++------- crates/forge_main/src/vscode.rs | 6 +- crates/forge_main/src/zsh/paste.rs | 10 +- crates/forge_main/src/zsh/plugin.rs | 60 +++++---- crates/forge_markdown_stream/src/renderer.rs | 3 +- crates/forge_markdown_stream/src/table.rs | 3 +- crates/forge_repo/src/agent.rs | 4 +- .../src/conversation/conversation_repo.rs | 17 ++- crates/forge_repo/src/provider/anthropic.rs | 22 ++-- crates/forge_repo/src/provider/bedrock.rs | 68 ++++++---- crates/forge_repo/src/provider/chat.rs | 3 +- crates/forge_repo/src/provider/google.rs | 9 +- crates/forge_repo/src/provider/openai.rs | 47 ++++--- .../provider/openai_responses/repository.rs | 3 +- .../src/provider/openai_responses/request.rs | 20 +-- .../src/provider/openai_responses/response.rs | 71 ++++++---- .../forge_repo/src/provider/provider_repo.rs | 23 ++-- crates/forge_repo/src/skill.rs | 18 ++- crates/forge_select/src/input.rs | 5 +- crates/forge_services/src/agent_registry.rs | 4 +- crates/forge_services/src/app_config.rs | 3 +- crates/forge_services/src/attachment.rs | 24 ++-- crates/forge_services/src/clipper.rs | 8 +- crates/forge_services/src/command.rs | 11 +- crates/forge_services/src/context_engine.rs | 7 +- crates/forge_services/src/fd.rs | 13 +- crates/forge_services/src/mcp/manager.rs | 6 +- crates/forge_services/src/mcp/service.rs | 40 +++--- crates/forge_services/src/provider_auth.rs | 15 ++- crates/forge_services/src/provider_service.rs | 3 +- crates/forge_services/src/template.rs | 7 +- .../src/tool_services/fs_patch.rs | 19 ++- .../src/tool_services/fs_read.rs | 3 +- .../src/tool_services/fs_search.rs | 12 +- .../src/tool_services/fs_write.rs | 3 +- .../src/tool_services/image_read.rs | 4 +- crates/forge_snaps/src/service.rs | 3 +- crates/forge_tracker/src/collect/posthog.rs | 3 +- crates/forge_walker/src/walker.rs | 25 ++-- 139 files changed, 1206 insertions(+), 835 deletions(-) diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index a056705761..b13c374b6e 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -241,8 +241,8 @@ impl< } async fn update_config(&self, ops: Vec) -> anyhow::Result<()> { - // Determine whether any op affects provider/model resolution before writing, - // so we can invalidate the agent cache afterwards. + // Determine whether any op affects provider/model resolution before + // writing, so we can invalidate the agent cache afterwards. let needs_agent_reload = ops .iter() .any(|op| matches!(op, forge_domain::ConfigOperation::SetSessionConfig(_))); diff --git a/crates/forge_app/src/agent.rs b/crates/forge_app/src/agent.rs index a640ba004e..f6ae464129 100644 --- a/crates/forge_app/src/agent.rs +++ b/crates/forge_app/src/agent.rs @@ -130,8 +130,9 @@ impl AgentExt for Agent { // Apply workflow compact configuration to agents if let Some(ref workflow_compact) = config.compact { - // Convert forge_config::Compact to forge_domain::Compact, then merge. - // Agent settings take priority over workflow settings. + // Convert forge_config::Compact to forge_domain::Compact, then + // merge. Agent settings take priority over workflow + // settings. let mut merged_compact = Compact { retention_window: workflow_compact.retention_window, eviction_window: workflow_compact.eviction_window.value(), @@ -169,7 +170,8 @@ impl AgentExt for Agent { exclude: config_reasoning.exclude, enabled: config_reasoning.enabled, }; - // Start from the agent's own settings and fill unset fields from config. + // Start from the agent's own settings and fill unset fields from + // config. let mut merged = agent.reasoning.clone().unwrap_or_default(); merged.merge(config_as_domain); // If the config explicitly disables reasoning, honour that override @@ -302,8 +304,8 @@ mod tests { // CURRENT BEHAVIOR: Due to merge order (workflow_compact merged with // agent.compact), agent's retention_window=0 overwrites workflow's 10 - // This is the documented behavior: "Agent settings take priority over workflow - // settings" + // This is the documented behavior: "Agent settings take priority over + // workflow settings" // Agent default has retention_window=0, which overwrites workflow's 10 assert_eq!( diff --git a/crates/forge_app/src/agent_executor.rs b/crates/forge_app/src/agent_executor.rs index fe92b7c7d4..97ae8e5a5b 100644 --- a/crates/forge_app/src/agent_executor.rs +++ b/crates/forge_app/src/agent_executor.rs @@ -63,8 +63,9 @@ impl> AgentEx .await? .ok_or(Error::ConversationNotFound { id: conversation_id })? } else { - // Create context with agent initiator since it's spawned by a parent agent - // This is crucial for GitHub Copilot billing optimization + // Create context with agent initiator since it's spawned by a + // parent agent This is crucial for GitHub Copilot + // billing optimization let context = forge_domain::Context::default().initiator("agent".to_string()); let conversation = Conversation::generate() .title(task.clone()) diff --git a/crates/forge_app/src/agent_provider_resolver.rs b/crates/forge_app/src/agent_provider_resolver.rs index 82e5a48197..8f4b981444 100644 --- a/crates/forge_app/src/agent_provider_resolver.rs +++ b/crates/forge_app/src/agent_provider_resolver.rs @@ -28,7 +28,8 @@ where // Load all agent definitions and find the one we need if let Some(agent) = self.0.get_agent(&agent_id).await? { - // If the agent definition has a provider, use it; otherwise use default + // If the agent definition has a provider, use it; otherwise use + // default agent.provider } else { // TODO: Needs review, should we throw an err here? diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index d53b3c5b7e..ff16db7a77 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -148,8 +148,8 @@ impl> ForgeAp let tracing_handler = TracingHandler::new(); let title_handler = TitleGenerationHandler::new(services.clone()); - // Build the on_end hook, conditionally adding PendingTodosHandler based on - // config + // Build the on_end hook, conditionally adding PendingTodosHandler based + // on config let on_end_hook = if forge_config.verify_todos { tracing_handler .clone() @@ -194,7 +194,8 @@ impl> ForgeAp let conversation = orch.get_conversation().clone(); let save_result = services.upsert_conversation(conversation).await; - // Send any error to the stream (prioritize dispatch error over save error) + // Send any error to the stream (prioritize dispatch error + // over save error) #[allow(clippy::collapsible_if)] if let Some(err) = dispatch_result.err().or(save_result.err()) { if let Err(e) = tx.send(Err(err)).await { @@ -306,7 +307,8 @@ impl> ForgeAp pub async fn get_all_provider_models(&self) -> Result> { let all_providers = self.services.get_all_providers().await?; - // Build one future per configured provider, preserving the error on failure. + // Build one future per configured provider, preserving the error on + // failure. let futures: Vec<_> = all_providers .into_iter() .filter_map(|any_provider| any_provider.into_configured()) diff --git a/crates/forge_app/src/command_generator.rs b/crates/forge_app/src/command_generator.rs index 122fbc2ec8..13e3c51e5e 100644 --- a/crates/forge_app/src/command_generator.rs +++ b/crates/forge_app/src/command_generator.rs @@ -229,7 +229,8 @@ mod tests { .map(|(path, is_dir)| File { path: path.clone(), is_dir: *is_dir }) .collect(); - // Sort: directories first (alphabetically), then files (alphabetically) + // Sort: directories first (alphabetically), then files + // (alphabetically) files.sort_by(|a, b| match (a.is_dir, b.is_dir) { (true, false) => std::cmp::Ordering::Less, (false, true) => std::cmp::Ordering::Greater, diff --git a/crates/forge_app/src/compact.rs b/crates/forge_app/src/compact.rs index 8affde4843..a62aad5cde 100644 --- a/crates/forge_app/src/compact.rs +++ b/crates/forge_app/src/compact.rs @@ -113,10 +113,10 @@ impl Compactor { // chains. After compaction, this consistency can break if the first // remaining assistant lacks reasoning. // - // Solution: Extract the LAST reasoning from compacted messages and inject it - // into the first assistant message after compaction. This preserves - // chain continuity while preventing exponential accumulation across - // multiple compactions. + // Solution: Extract the LAST reasoning from compacted messages and + // inject it into the first assistant message after compaction. + // This preserves chain continuity while preventing exponential + // accumulation across multiple compactions. // // Example: [U, A+r, U, A+r, U, A] → compact → [U-summary, A+r, U, A] // └─from last @@ -133,8 +133,8 @@ impl Compactor { _ => None, }); - // Accumulate usage from all messages in the compaction range before they are - // destroyed + // Accumulate usage from all messages in the compaction range before + // they are destroyed let compacted_usage = context.messages.get(start..=end).and_then(|slice| { slice .iter() @@ -143,7 +143,8 @@ impl Compactor { .reduce(|a, b| a.accumulate(&b)) }); - // Replace the range with the summary, transferring the accumulated usage + // Replace the range with the summary, transferring the accumulated + // usage let mut summary_entry = MessageEntry::from(ContextMessage::user(summary, None)); summary_entry.usage = compacted_usage; context @@ -289,7 +290,8 @@ mod tests { let context = compactor.compress_single_sequence(context, (0, 2)).unwrap(); - // Verify reasoning didn't accumulate - should still be just 1 reasoning block + // Verify reasoning didn't accumulate - should still be just 1 reasoning + // block let first_assistant = context .messages .iter() @@ -318,7 +320,8 @@ mod tests { ..Default::default() }]; - // Most recent message in range has empty reasoning, earlier has non-empty + // Most recent message in range has empty reasoning, earlier has + // non-empty let context = Context::default() .add_message(ContextMessage::user("M1", None)) .add_message(ContextMessage::assistant( @@ -647,8 +650,9 @@ mod tests { "Summary message should carry accumulated usage from compacted messages" ); - // accumulate_usage() must sum both the compacted range usage (on the summary - // message) and the surviving outside_usage — total = inside + inside2 + outside + // accumulate_usage() must sum both the compacted range usage (on the + // summary message) and the surviving outside_usage — total = + // inside + inside2 + outside let expected_total_usage = Usage { total_tokens: TokenCount::Actual(100000), prompt_tokens: TokenCount::Actual(90000), @@ -855,17 +859,19 @@ mod tests { // - Safe threshold (89.6K): ~95K tokens, SHOULD compact (true) // // At turn 2: - // - Unsafe threshold (100K): ~110K tokens, SHOULD compact (true) - but too - // late! + // - Unsafe threshold (100K): ~110K tokens, SHOULD compact (true) - but + // too late! // - Safe threshold (89.6K): ~110K tokens, already compacted at turn 1 - // Verify that safe threshold triggers at turn 1 (providing early warning) + // Verify that safe threshold triggers at turn 1 (providing early + // warning) let safe_token_count_turn1 = 95_000; // Approximate let safe_should_compact_turn1 = safe_compact.should_compact(&safe_context, safe_token_count_turn1); - // The key fix: safe threshold (89.6K) triggers at ~95K, while unsafe (100K) - // doesn't This provides a safety margin before we hit the 128K limit + // The key fix: safe threshold (89.6K) triggers at ~95K, while unsafe + // (100K) doesn't This provides a safety margin before we hit + // the 128K limit assert!( safe_should_compact_turn1 || safe_token_count_turn1 < 89_600, "Safe threshold (89.6K) should trigger compaction at ~95K tokens to provide safety margin" @@ -876,8 +882,8 @@ mod tests { let final_unsafe = unsafe_context.token_count_approx(); let final_safe = safe_context.token_count_approx(); - // Both should be identical since we're just testing threshold logic, not actual - // compaction + // Both should be identical since we're just testing threshold logic, + // not actual compaction assert_eq!( final_unsafe, final_safe, "Both contexts should have same token count" diff --git a/crates/forge_app/src/dto/anthropic/request.rs b/crates/forge_app/src/dto/anthropic/request.rs index 72aa4496ec..53e1870b3e 100644 --- a/crates/forge_app/src/dto/anthropic/request.rs +++ b/crates/forge_app/src/dto/anthropic/request.rs @@ -129,8 +129,9 @@ impl TryFrom for Request { // a positive effort / `max_tokens` still emit reasoning on the wire. let reasoning_on = request.is_reasoning_supported(); let (thinking, output_config) = if reasoning_on && let Some(reasoning) = request.reasoning { - // Adaptive thinking on 4.7 hides reasoning content by default; opting - // into reasoning should surface it unless the caller set `exclude`. + // Adaptive thinking on 4.7 hides reasoning content by default; + // opting into reasoning should surface it unless the + // caller set `exclude`. let adaptive_display = if reasoning.exclude == Some(true) { Some(ThinkingDisplay::Omitted) } else { @@ -185,7 +186,8 @@ impl TryFrom for Request { output_config, output_format: request.response_format.and_then(|rf| match rf { forge_domain::ResponseFormat::Text => { - // Anthropic doesn't have a "text" output format, so we skip it + // Anthropic doesn't have a "text" output format, so we skip + // it None } forge_domain::ResponseFormat::JsonSchema(schema) => { @@ -260,7 +262,8 @@ impl TryFrom for Message { forge_domain::Role::User => Message { role: Role::User, content }, forge_domain::Role::Assistant => Message { role: Role::Assistant, content }, forge_domain::Role::System => { - // note: Anthropic doesn't support system role messages and they're already + // note: Anthropic doesn't support system role messages + // and they're already // filtered out. so this state is unreachable. return Err( forge_domain::Error::UnsupportedRole("System".to_string()).into() @@ -285,7 +288,8 @@ impl Message { *content = std::mem::take(content).cached(false); } - // If enabling cache, set cache control on the last cacheable content item + // If enabling cache, set cache control on the last cacheable content + // item if enable_cache && let Some(last_cacheable_idx) = self.content @@ -592,7 +596,8 @@ mod tests { #[test] fn test_reasoning_max_tokens_and_effort_emit_both() { - // Effort and budget are independent knobs — neither should hide the other. + // Effort and budget are independent knobs — neither should hide the + // other. let fixture = Context::default().reasoning(ReasoningConfig { effort: Some(forge_domain::Effort::Low), enabled: Some(true), @@ -735,9 +740,9 @@ mod tests { #[test] fn test_reasoning_enabled_none_with_max_tokens_still_emits_thinking() { - // Matches the domain's `is_reasoning_supported` rule: enabled: None with a - // positive budget counts as on, so inherited/merged configs don't silently - // disable reasoning on the wire. + // Matches the domain's `is_reasoning_supported` rule: enabled: None + // with a positive budget counts as on, so inherited/merged + // configs don't silently disable reasoning on the wire. let fixture = Context::default().reasoning(ReasoningConfig { enabled: None, max_tokens: Some(8000), diff --git a/crates/forge_app/src/dto/anthropic/response.rs b/crates/forge_app/src/dto/anthropic/response.rs index 6d3b89c759..801fa5de18 100644 --- a/crates/forge_app/src/dto/anthropic/response.rs +++ b/crates/forge_app/src/dto/anthropic/response.rs @@ -164,9 +164,11 @@ impl From for forge_domain::Usage { fn from(usage: Usage) -> Self { // Anthropic token breakdown: // - input_tokens: tokens NOT from cache (billed at full price) - // - cache_creation_input_tokens: tokens written to cache (billed at full price + // - cache_creation_input_tokens: tokens written to cache (billed at + // full price // + write cost) - // - cache_read_input_tokens: tokens read from cache (billed at 90% discount) + // - cache_read_input_tokens: tokens read from cache (billed at 90% + // discount) // Total input = input_tokens + cache_creation_input_tokens + // cache_read_input_tokens @@ -320,7 +322,8 @@ impl TryFrom for ChatCompletionMessage { ChatCompletionMessage::try_from(content_block)? } Event::MessageStart { message } => { - // Extract usage from MessageStart - this contains input token counts + // Extract usage from MessageStart - this contains input token + // counts ChatCompletionMessage::assistant(Content::part("")).usage(message.usage) } Event::MessageDelta { delta, usage } => { @@ -332,7 +335,8 @@ impl TryFrom for ChatCompletionMessage { return Err(error.into()); } Event::Ping { cost: Some(cost) } => { - // OpenCode Zen sends cost in a ping event at the end of the stream + // OpenCode Zen sends cost in a ping event at the end of the + // stream let cost_value = match cost { StringOrF64::Number(n) => n, StringOrF64::String(s) => s.parse().unwrap_or(0.0), @@ -395,8 +399,9 @@ impl TryFrom for ChatCompletionMessage { ) } ContentBlock::ToolUse { id, name, input } => { - // note: We've to check if the input is empty or null. else we end up adding - // empty object `{}` as prefix to tool args. + // note: We've to check if the input is empty or null. else we + // end up adding empty object `{}` as prefix to + // tool args. let is_empty = input.is_null() || input.as_object().is_some_and(|map| map.is_empty()); ChatCompletionMessage::assistant(Content::part("")).add_tool_call(ToolCallPart { @@ -565,8 +570,8 @@ mod tests { let expected_prompt = TokenCount::Actual(100 + 200 + 300); assert_eq!(actual.prompt_tokens, expected_prompt); - // cached_tokens should only include cache reads (tokens that benefited from - // caching) + // cached_tokens should only include cache reads (tokens that benefited + // from caching) let expected_cached = TokenCount::Actual(300); assert_eq!(actual.cached_tokens, expected_cached); diff --git a/crates/forge_app/src/dto/anthropic/transforms/enforce_schema.rs b/crates/forge_app/src/dto/anthropic/transforms/enforce_schema.rs index 43c7987e57..ab3d90abcc 100644 --- a/crates/forge_app/src/dto/anthropic/transforms/enforce_schema.rs +++ b/crates/forge_app/src/dto/anthropic/transforms/enforce_schema.rs @@ -37,7 +37,8 @@ impl Transformer for EnforceStrictObjectSchema { if let Some(OutputFormat::JsonSchema { schema }) = request.output_format.take() { // Convert schema to JSON value for normalization if let Ok(mut schema_value) = serde_json::to_value(&schema) { - // Use non-strict mode (false) for Anthropic - only adds additionalProperties + // Use non-strict mode (false) for Anthropic - only adds + // additionalProperties enforce_strict_schema(&mut schema_value, false); // Convert back to RootSchema diff --git a/crates/forge_app/src/dto/anthropic/transforms/mcp_tool_names.rs b/crates/forge_app/src/dto/anthropic/transforms/mcp_tool_names.rs index 31a82f3f5f..d709e21c93 100644 --- a/crates/forge_app/src/dto/anthropic/transforms/mcp_tool_names.rs +++ b/crates/forge_app/src/dto/anthropic/transforms/mcp_tool_names.rs @@ -68,7 +68,8 @@ mod tests { #[test] fn test_handles_server_name_containing_tool_substring() { // Server named "my_tool_server", tool named "action" - // rfind gives the LAST `_tool_`, so server = "my_tool_server", tool = "action" + // rfind gives the LAST `_tool_`, so server = "my_tool_server", tool = + // "action" let actual = to_claude_code_format("mcp_my_tool_server_tool_action"); let expected = "mcp__my_tool_server__action"; assert_eq!(actual, expected); @@ -76,7 +77,8 @@ mod tests { #[test] fn test_leaves_already_converted_names_unchanged() { - // mcp__ prefix means it was already converted; no `_tool_` in sanitized names + // mcp__ prefix means it was already converted; no `_tool_` in sanitized + // names let actual = to_claude_code_format("mcp__github__create_issue"); let expected = "mcp__github__create_issue"; assert_eq!(actual, expected); diff --git a/crates/forge_app/src/dto/anthropic/transforms/set_cache.rs b/crates/forge_app/src/dto/anthropic/transforms/set_cache.rs index 0f1598bec7..3a5a2a6bc6 100644 --- a/crates/forge_app/src/dto/anthropic/transforms/set_cache.rs +++ b/crates/forge_app/src/dto/anthropic/transforms/set_cache.rs @@ -70,7 +70,8 @@ mod tests { ) -> String { let mut messages = Vec::new(); - // Add system messages to the regular messages array for Anthropic format + // Add system messages to the regular messages array for Anthropic + // format for c in system_messages.chars() { match c { 's' => messages.push( diff --git a/crates/forge_app/src/dto/google/request.rs b/crates/forge_app/src/dto/google/request.rs index ee2ce1cf82..6fb53864bf 100644 --- a/crates/forge_app/src/dto/google/request.rs +++ b/crates/forge_app/src/dto/google/request.rs @@ -322,8 +322,8 @@ impl From for Request { }; // Convert messages (excluding system messages) - // Group consecutive tool results into single Content objects to match Google's - // API requirements + // Group consecutive tool results into single Content objects to match + // Google's API requirements let mut contents: Vec = Vec::new(); let mut pending_tool_parts: Vec = Vec::new(); @@ -564,8 +564,8 @@ mod tests { #[test] fn test_tool_call_args_serialization() { - // Create a ToolCallFull with Unparsed JSON arguments (as it would come from - // API) + // Create a ToolCallFull with Unparsed JSON arguments (as it would come + // from API) let tool_call = ToolCallFull { name: ToolName::new("patch"), call_id: None, @@ -649,8 +649,8 @@ mod tests { fn test_consecutive_tool_results_grouped() { use forge_domain::{Context, ContextMessage, ModelId}; - // Create a context with multiple consecutive tool results (simulating 13 read - // calls) + // Create a context with multiple consecutive tool results (simulating + // 13 read calls) let mut context = Context::default(); // Add initial user message @@ -675,7 +675,8 @@ mod tests { // Verify structure: // 1. First content: user message - // 2. Second content: assistant message (might be empty and filtered out) + // 2. Second content: assistant message (might be empty and filtered + // out) // 3. Third content: ALL 13 tool results grouped together // Find the content with tool results @@ -756,7 +757,8 @@ mod tests { }) .collect(); - // Should have 2 separate Content objects for the 2 non-consecutive tool results + // Should have 2 separate Content objects for the 2 non-consecutive tool + // results assert_eq!( contents_with_tool_results.len(), 2, @@ -939,7 +941,8 @@ mod tests { "Schema should still contain other properties" ); - // Verify additionalProperties is also removed by Gemini sanitization + // Verify additionalProperties is also removed by Gemini + // sanitization assert!( !obj.contains_key("additionalProperties"), "additionalProperties should be removed by Gemini sanitization" diff --git a/crates/forge_app/src/dto/google/response.rs b/crates/forge_app/src/dto/google/response.rs index 404f2342fc..8355a3d2d9 100644 --- a/crates/forge_app/src/dto/google/response.rs +++ b/crates/forge_app/src/dto/google/response.rs @@ -435,7 +435,8 @@ impl TryFrom for ChatCompletionMessage { ), ), Part::InlineData { .. } => { - // For now, skip inline data in responses (it's typically for inputs) + // For now, skip inline data in responses (it's typically for + // inputs) Ok(ChatCompletionMessage::assistant( forge_domain::Content::part(""), )) diff --git a/crates/forge_app/src/dto/openai/error.rs b/crates/forge_app/src/dto/openai/error.rs index 6c2c84c4f3..f88f285a68 100644 --- a/crates/forge_app/src/dto/openai/error.rs +++ b/crates/forge_app/src/dto/openai/error.rs @@ -136,7 +136,8 @@ mod tests { #[test] fn test_get_code_deep_nested() { - // Test with an error that has no direct code but has an inner error with a code + // Test with an error that has no direct code but has an inner error + // with a code let error_code = ErrorCode::String("ERR_STREAM_PREMATURE_CLOSE".to_string()); // Use derived setters for cleaner initialization diff --git a/crates/forge_app/src/dto/openai/model.rs b/crates/forge_app/src/dto/openai/model.rs index 78d2001a8c..3e82d9d95c 100644 --- a/crates/forge_app/src/dto/openai/model.rs +++ b/crates/forge_app/src/dto/openai/model.rs @@ -150,8 +150,8 @@ mod tests { #[tokio::test] async fn test_deserialize_model_with_numeric_pricing() { - // This reproduces the issue where Chutes API returns numeric pricing instead of - // strings + // This reproduces the issue where Chutes API returns numeric pricing + // instead of strings let fixture = load_fixture("model_numeric_pricing.json").await; let actual = serde_json::from_value::(fixture).unwrap(); @@ -226,8 +226,8 @@ mod tests { #[tokio::test] async fn test_chutes_api_response_format() { - // This simulates the actual Chutes API response format that was causing the - // issue + // This simulates the actual Chutes API response format that was causing + // the issue let fixture = load_fixture("chutes_api_response.json").await; let actual = serde_json::from_value::(fixture).unwrap(); @@ -321,7 +321,8 @@ mod tests { let domain_model: forge_domain::Model = model.into(); - // When supported_parameters is None, capabilities should be None (unknown) + // When supported_parameters is None, capabilities should be None + // (unknown) assert_eq!(domain_model.tools_supported, None); assert_eq!(domain_model.supports_parallel_tool_calls, None); assert_eq!(domain_model.supports_reasoning, None); diff --git a/crates/forge_app/src/dto/openai/request.rs b/crates/forge_app/src/dto/openai/request.rs index 309aeaca09..65784dcb60 100644 --- a/crates/forge_app/src/dto/openai/request.rs +++ b/crates/forge_app/src/dto/openai/request.rs @@ -324,7 +324,8 @@ impl From for Tool { name: value.name.to_string(), parameters: { let mut params = serde_json::to_value(value.input_schema).unwrap(); - // Ensure OpenAI compatibility by adding properties field if missing + // Ensure OpenAI compatibility by adding properties field if + // missing if let Some(obj) = params.as_object_mut() && obj.get("type") == Some(&serde_json::Value::String("object".to_string())) && !obj.contains_key("properties") @@ -846,9 +847,11 @@ mod tests { } #[test] fn test_tool_definition_conversion_missing_properties() { - // Test case where input_schema is an object type but missing properties field + // Test case where input_schema is an object type but missing properties + // field let fixture = { - // In schemars 1.0, Schema wraps serde_json::Value, so we create JSON directly + // In schemars 1.0, Schema wraps serde_json::Value, so we create + // JSON directly let schema_value = serde_json::json!({ "$schema": "http://json-schema.org/draft-07/schema#", "title": "Null", diff --git a/crates/forge_app/src/dto/openai/response.rs b/crates/forge_app/src/dto/openai/response.rs index 5750b34bb2..967a22b9c2 100644 --- a/crates/forge_app/src/dto/openai/response.rs +++ b/crates/forge_app/src/dto/openai/response.rs @@ -526,8 +526,9 @@ impl TryFrom for ChatCompletionMessage { } let mut default_response = ChatCompletionMessage::assistant(Content::full("")); - // No choices – this can happen with Ollama/LMStudio streaming where the final - // chunk only contains usage information. + // No choices – this can happen with Ollama/LMStudio + // streaming where the final chunk only + // contains usage information. if let Some(u) = usage { default_response.usage = Some(u.into()); } @@ -667,8 +668,9 @@ mod tests { #[tokio::test] async fn test_kimi_k2_both_reasoning_keys_event() { - // moonshotai/Kimi-K2.5-TEE emits both "reasoning" and "reasoning_content" - // in the same delta object. This must parse without a duplicate_field error. + // moonshotai/Kimi-K2.5-TEE emits both "reasoning" and + // "reasoning_content" in the same delta object. This must parse + // without a duplicate_field error. let fixture = load_fixture("chutes_completion_response.json").await; let actual = serde_json::from_value::(fixture); assert!(actual.is_ok(), "Failed to parse: {:?}", actual.err()); @@ -943,7 +945,8 @@ mod tests { let actual: Usage = fixture_upstream_wins.into(); assert_eq!(actual.cost, Some(0.005)); - // Priority 3: partial costs are summed when upstream_inference_cost is None + // Priority 3: partial costs are summed when upstream_inference_cost is + // None let fixture_partial_sum = ResponseUsage { prompt_tokens: 100, completion_tokens: 50, @@ -960,8 +963,8 @@ mod tests { let actual: Usage = fixture_partial_sum.into(); assert_eq!(actual.cost, Some(0.005)); - // Priority 4: when upstream_inference_cost is 0 then compute it from other - // parameters. + // Priority 4: when upstream_inference_cost is 0 then compute it from + // other parameters. let fixture = ResponseUsage { prompt_tokens: 100, completion_tokens: 50, diff --git a/crates/forge_app/src/dto/openai/transformers/ensure_system_first.rs b/crates/forge_app/src/dto/openai/transformers/ensure_system_first.rs index c99885a562..8d2487a09b 100644 --- a/crates/forge_app/src/dto/openai/transformers/ensure_system_first.rs +++ b/crates/forge_app/src/dto/openai/transformers/ensure_system_first.rs @@ -43,7 +43,8 @@ impl Transformer for MergeSystemMessages { .join("\n\n"); if combined_content.is_empty() { - // All system messages had no content, don't create empty system message + // All system messages had no content, don't create empty + // system message rest } else { let mut result = vec![Message { diff --git a/crates/forge_app/src/dto/openai/transformers/github_copilot_reasoning.rs b/crates/forge_app/src/dto/openai/transformers/github_copilot_reasoning.rs index 81436c9562..cec48c7b1b 100644 --- a/crates/forge_app/src/dto/openai/transformers/github_copilot_reasoning.rs +++ b/crates/forge_app/src/dto/openai/transformers/github_copilot_reasoning.rs @@ -36,7 +36,8 @@ impl Transformer for GitHubCopilotReasoning { message.reasoning_text = reasoning_text; message.reasoning_opaque = reasoning_opaque; - // Remove reasoning_details array (GitHub Copilot doesn't accept it) + // Remove reasoning_details array (GitHub Copilot doesn't + // accept it) message.reasoning_details = None; } } diff --git a/crates/forge_app/src/dto/openai/transformers/make_openai_compat.rs b/crates/forge_app/src/dto/openai/transformers/make_openai_compat.rs index d1ccd5fa77..12d94dcfdb 100644 --- a/crates/forge_app/src/dto/openai/transformers/make_openai_compat.rs +++ b/crates/forge_app/src/dto/openai/transformers/make_openai_compat.rs @@ -29,11 +29,13 @@ impl Transformer for MakeOpenAiCompat { .is_some_and(|tools| !tools.is_empty()); if !tools_present { - // drop `parallel_tool_calls` field if tools are not passed to the request. + // drop `parallel_tool_calls` field if tools are not passed to the + // request. request.parallel_tool_calls = None; } - // OpenAI has deprecated `max_tokens`, now it is `max_completion_tokens`. + // OpenAI has deprecated `max_tokens`, now it is + // `max_completion_tokens`. request.max_completion_tokens = request.max_tokens.take(); request diff --git a/crates/forge_app/src/dto/openai/transformers/normalize_tool_schema.rs b/crates/forge_app/src/dto/openai/transformers/normalize_tool_schema.rs index b0e0df84ef..78b2deef72 100644 --- a/crates/forge_app/src/dto/openai/transformers/normalize_tool_schema.rs +++ b/crates/forge_app/src/dto/openai/transformers/normalize_tool_schema.rs @@ -26,7 +26,8 @@ impl Transformer for NormalizeToolSchema { if let Some(tools) = request.tools.as_mut() { for tool in tools.iter_mut() { if let Some(obj) = tool.function.parameters.as_object_mut() { - // Remove tool usage description and title from parameters property + // Remove tool usage description and title from parameters + // property obj.remove("description"); obj.remove("title"); } diff --git a/crates/forge_app/src/dto/openai/transformers/pipeline.rs b/crates/forge_app/src/dto/openai/transformers/pipeline.rs index f8009763c5..f142837a8e 100644 --- a/crates/forge_app/src/dto/openai/transformers/pipeline.rs +++ b/crates/forge_app/src/dto/openai/transformers/pipeline.rs @@ -48,8 +48,8 @@ impl Transformer for ProviderPipeline<'_> { let provider = self.provider; let merge_system_messages = self.merge_system_messages; - // Z.ai transformer must run before MakeOpenAiCompat which removes reasoning - // field + // Z.ai transformer must run before MakeOpenAiCompat which removes + // reasoning field let zai_thinking = SetZaiThinking.when(move |_| is_zai_provider(provider)); let or_transformers = DefaultTransformation::::new() @@ -482,8 +482,9 @@ mod tests { fn test_merge_system_messages_flag_merges_for_any_provider() { use crate::dto::openai::{Message, MessageContent, Role}; - // Use a plain OpenAI provider — it would NOT merge system messages by default. - // The global `merge_system_messages = true` flag must trigger the merge. + // Use a plain OpenAI provider — it would NOT merge system messages by + // default. The global `merge_system_messages = true` flag must + // trigger the merge. let provider = openai("openai-key"); let fixture = Request::default().messages(vec![ Message { @@ -864,7 +865,8 @@ mod tests { let mut pipeline = ProviderPipeline::new(&provider, false); let actual = pipeline.transform(fixture); - // Thought signature should be stripped for gemini-2 models (not gemini-3) + // Thought signature should be stripped for gemini-2 models (not + // gemini-3) let messages = actual.messages.unwrap(); assert!(messages[0].extra_content.is_none()); } diff --git a/crates/forge_app/src/dto/openai/transformers/reasoning_content.rs b/crates/forge_app/src/dto/openai/transformers/reasoning_content.rs index 6d1122cbe1..878310b8d9 100644 --- a/crates/forge_app/src/dto/openai/transformers/reasoning_content.rs +++ b/crates/forge_app/src/dto/openai/transformers/reasoning_content.rs @@ -30,7 +30,8 @@ impl Transformer for ReasoningContent { // Set flat field message.reasoning_content = reasoning_content; - // Remove reasoning_details array because target providers do not accept it + // Remove reasoning_details array because target providers + // do not accept it message.reasoning_details = None; } } diff --git a/crates/forge_app/src/dto/openai/transformers/set_cache.rs b/crates/forge_app/src/dto/openai/transformers/set_cache.rs index 61052c3d3c..93b4946a96 100644 --- a/crates/forge_app/src/dto/openai/transformers/set_cache.rs +++ b/crates/forge_app/src/dto/openai/transformers/set_cache.rs @@ -24,7 +24,8 @@ impl Transformer for SetCache { return request; } - // Remove cache control from second-to-last message (when there are 3+ messages) + // Remove cache control from second-to-last message (when there are + // 3+ messages) if len >= 3 && let Some(message) = messages.get_mut(len - 2) && let Some(ref content) = message.content diff --git a/crates/forge_app/src/dto/openai/transformers/set_reasoning_effort.rs b/crates/forge_app/src/dto/openai/transformers/set_reasoning_effort.rs index f36c5ebf85..ed0d355165 100644 --- a/crates/forge_app/src/dto/openai/transformers/set_reasoning_effort.rs +++ b/crates/forge_app/src/dto/openai/transformers/set_reasoning_effort.rs @@ -167,7 +167,8 @@ mod tests { #[test] fn test_reasoning_with_budget_defaults_to_medium_effort() { // max_tokens (budget) is independent from effort; when only budget is - // set and enabled=true, the transformer falls back to the default effort. + // set and enabled=true, the transformer falls back to the default + // effort. let fixture = Request::default().reasoning(ReasoningConfig { enabled: Some(true), effort: None, @@ -184,7 +185,8 @@ mod tests { #[test] fn test_reasoning_with_budget_high_defaults_to_medium_effort() { - // Even a large budget does not elevate the effort; use explicit effort instead. + // Even a large budget does not elevate the effort; use explicit effort + // instead. let fixture = Request::default().reasoning(ReasoningConfig { enabled: Some(true), effort: None, @@ -201,7 +203,8 @@ mod tests { #[test] fn test_effort_takes_precedence_over_budget() { - // When both effort and max_tokens are set, effort should take precedence + // When both effort and max_tokens are set, effort should take + // precedence let fixture = Request::default().reasoning(ReasoningConfig { enabled: Some(true), effort: Some(Effort::High), diff --git a/crates/forge_app/src/dto/openai/transformers/tool_choice.rs b/crates/forge_app/src/dto/openai/transformers/tool_choice.rs index 1ac8810bcf..12996e6256 100644 --- a/crates/forge_app/src/dto/openai/transformers/tool_choice.rs +++ b/crates/forge_app/src/dto/openai/transformers/tool_choice.rs @@ -17,8 +17,8 @@ impl Transformer for SetToolChoice { fn transform(&mut self, mut request: Self::Value) -> Self::Value { // Only set tool_choice if there are tools defined - // This prevents "Function calling config is set without function_declarations" - // error + // This prevents "Function calling config is set without + // function_declarations" error if request .tools .as_ref() diff --git a/crates/forge_app/src/file_tracking.rs b/crates/forge_app/src/file_tracking.rs index 5300f88234..d722bf5451 100644 --- a/crates/forge_app/src/file_tracking.rs +++ b/crates/forge_app/src/file_tracking.rs @@ -41,7 +41,8 @@ impl FileChangeDetector { /// if unreadable) pub async fn detect(&self, metrics: &Metrics, parallel_file_reads: usize) -> Vec { let fs = self.fs_read_service.clone(); - // Collect into owned data upfront so the stream futures are 'static-safe + // Collect into owned data upfront so the stream futures are + // 'static-safe let entries: Vec<(std::path::PathBuf, Option)> = metrics .file_operations .iter() @@ -331,7 +332,8 @@ mod tests { ); // Even though displayed content differs from raw, the hash comparison - // uses the raw-based content_hash from ReadOutput, so no false positive. + // uses the raw-based content_hash from ReadOutput, so no false + // positive. let actual = detector.detect(&metrics, 64).await; let expected = vec![]; diff --git a/crates/forge_app/src/fmt/fmt_input.rs b/crates/forge_app/src/fmt/fmt_input.rs index f483f1d57e..38c3ef8438 100644 --- a/crates/forge_app/src/fmt/fmt_input.rs +++ b/crates/forge_app/src/fmt/fmt_input.rs @@ -36,7 +36,8 @@ impl FormatContent for ToolCatalog { let title = match (path.exists(), input.overwrite) { (true, true) => "Overwrite", (true, false) => { - // Case: file exists but overwrite is false then we throw error from tool, + // Case: file exists but overwrite is false then we + // throw error from tool, // so it's good idea to not print anything on CLI. return None; } diff --git a/crates/forge_app/src/fmt/todo_fmt.rs b/crates/forge_app/src/fmt/todo_fmt.rs index 7b676a21a2..a8f4828b69 100644 --- a/crates/forge_app/src/fmt/todo_fmt.rs +++ b/crates/forge_app/src/fmt/todo_fmt.rs @@ -67,7 +67,8 @@ pub(crate) fn format_todos_diff(before: &[Todo], after: &[Todo]) -> String { // items, or the removed rendering for items that were dropped. for before_todo in before { if let Some(after_todo) = after_map.get(before_todo.id.as_str()).copied() { - // Item still exists — render with bold/dim based on whether it changed. + // Item still exists — render with bold/dim based on whether it + // changed. let is_changed = before_todo.status != after_todo.status || before_todo.content != after_todo.content; let line_style = if is_changed { @@ -276,8 +277,9 @@ mod tests { #[test] fn test_format_todos_preserves_insertion_order() { - // Items are given in insertion order: Second was added first, First second. - // Output must reflect that insertion order, not alphabetical or id-sorted. + // Items are given in insertion order: Second was added first, First + // second. Output must reflect that insertion order, not + // alphabetical or id-sorted. let setup = vec![ fixture_todo("Second", "2", TodoStatus::Pending), fixture_todo("First", "1", TodoStatus::Pending), diff --git a/crates/forge_app/src/git_app.rs b/crates/forge_app/src/git_app.rs index 1a1939ad20..fa8945e857 100644 --- a/crates/forge_app/src/git_app.rs +++ b/crates/forge_app/src/git_app.rs @@ -269,7 +269,8 @@ impl> GitApp< let staged_diff = staged_diff.context("Failed to get staged changes")?; let unstaged_diff = unstaged_diff.context("Failed to get unstaged changes")?; - // Use staged changes if available, otherwise fall back to unstaged changes + // Use staged changes if available, otherwise fall back to unstaged + // changes let has_staged_files = !staged_diff.output.stdout.trim().is_empty(); let diff_output = if has_staged_files { staged_diff @@ -308,9 +309,10 @@ impl> GitApp< )?; let agent_provider_resolver = AgentProviderResolver::new(self.services.clone()); - // Resolve provider and model: commit config takes priority over agent defaults. - // If the configured provider is unavailable (e.g. logged out), fall back to the - // agent's provider/model with a warning. + // Resolve provider and model: commit config takes priority over agent + // defaults. If the configured provider is unavailable (e.g. + // logged out), fall back to the agent's provider/model with a + // warning. let (provider, model) = match commit_config { Some(mc) => match self.services.get_provider(mc.provider).await { Ok(provider) => match self.services.refresh_provider_credential(provider).await { @@ -367,12 +369,13 @@ impl> GitApp< let stream = self.services.chat(&model, context, provider).await?; let message = stream.into_full(false).await?; - // Parse the response - try JSON first (structured output), fallback to plain - // text + // Parse the response - try JSON first (structured output), fallback to + // plain text let commit_message = match serde_json::from_str::(&message.content) { Ok(response) => response.commit_message, Err(_) => { - // Fallback: Some providers don't support structured output, treat as plain text + // Fallback: Some providers don't support structured output, + // treat as plain text message.content.trim().to_string() } }; diff --git a/crates/forge_app/src/hooks/doom_loop.rs b/crates/forge_app/src/hooks/doom_loop.rs index 3515b74e7b..581704c860 100644 --- a/crates/forge_app/src/hooks/doom_loop.rs +++ b/crates/forge_app/src/hooks/doom_loop.rs @@ -130,12 +130,13 @@ impl DoomLoopDetector { // For a partial match, we consider it as the start of a new repetition let mut check_len = total_len; - // Special case: if total length is not evenly divisible by pattern_length, - // we have a partial match at the end + // Special case: if total length is not evenly divisible by + // pattern_length, we have a partial match at the end if !total_len.is_multiple_of(pattern_length) { let partial_len = total_len % pattern_length; - // Check if the partial segment matches the start of what would be the pattern - // We need to look back to find what the pattern would be + // Check if the partial segment matches the start of what would be + // the pattern We need to look back to find what the + // pattern would be if total_len < pattern_length + partial_len { return 0; } @@ -596,7 +597,8 @@ mod tests { let conversation = create_conversation_with_messages(vec![msg1, msg2, msg3]); - // Third consecutive identical call - should be caught by consecutive check + // Third consecutive identical call - should be caught by consecutive + // check let actual = detector.detect_from_conversation(&conversation); assert_eq!(actual, Some(3)); @@ -711,8 +713,8 @@ mod tests { let actual = detector.detect_from_conversation(&conversation); // Should detect the [4,5][4,5][4,5] pattern at the end - // The detector looks for the longest repeating pattern, starting from the most - // recent calls + // The detector looks for the longest repeating pattern, starting from + // the most recent calls // The pattern [4,5] repeats 3 times at the end assert_eq!(actual, Some(3)); } @@ -779,7 +781,8 @@ mod tests { let conv = create_conversation_with_messages(messages.clone()); assert_eq!(detector.detect_from_conversation(&conv), None); - // Step 9: [1,2,3,4,5,4,6,4,5] - no loop yet (only 1.5 repetitions of [4,5]) + // Step 9: [1,2,3,4,5,4,6,4,5] - no loop yet (only 1.5 repetitions of + // [4,5]) messages.push(create_assistant_message(&tool_5)); let conv = create_conversation_with_messages(messages.clone()); assert_eq!(detector.detect_from_conversation(&conv), None); @@ -799,8 +802,8 @@ mod tests { let conv = create_conversation_with_messages(messages.clone()); assert_eq!(detector.detect_from_conversation(&conv), None); - // Step 13: [1,2,3,4,5,4,6,4,5,4,5,4,5] - [4,5] pattern now repeats 3 times at - // end + // Step 13: [1,2,3,4,5,4,6,4,5,4,5,4,5] - [4,5] pattern now repeats 3 + // times at end messages.push(create_assistant_message(&tool_5)); let conv = create_conversation_with_messages(messages.clone()); diff --git a/crates/forge_app/src/hooks/pending_todos.rs b/crates/forge_app/src/hooks/pending_todos.rs index bad2b44fa6..5c38bda08e 100644 --- a/crates/forge_app/src/hooks/pending_todos.rs +++ b/crates/forge_app/src/hooks/pending_todos.rs @@ -60,8 +60,8 @@ impl EventHandle> for PendingTodosHandler { // This prevents duplicate reminders while still allowing new reminders // when todos change (e.g., some completed but others still pending) let should_add_reminder = if let Some(context) = &conversation.context { - // Find the most recent reminder message by looking for the template content - // pattern + // Find the most recent reminder message by looking for the template + // content pattern let last_reminder_todos: Option> = context .messages .iter() @@ -236,7 +236,8 @@ mod tests { let after_first = conversation.context.as_ref().unwrap().messages.len(); assert_eq!(after_first, 1); - // Second call with the same pending todos should NOT add another reminder + // Second call with the same pending todos should NOT add another + // reminder handler.handle(&event, &mut conversation).await.unwrap(); let after_second = conversation.context.as_ref().unwrap().messages.len(); assert_eq!(after_second, 1); // Still 1, no duplicate diff --git a/crates/forge_app/src/hooks/title_generation.rs b/crates/forge_app/src/hooks/title_generation.rs index 5984ccab52..66789fab4c 100644 --- a/crates/forge_app/src/hooks/title_generation.rs +++ b/crates/forge_app/src/hooks/title_generation.rs @@ -65,9 +65,10 @@ impl EventHandle> for TitleGenerationHa ) .reasoning(event.agent.reasoning.clone()); - // `or_insert_with` holds the shard lock for its entire call. Any occupied - // entry — InProgress, Awaiting, or Done — is left untouched, so at most - // one task is ever spawned per conversation id. + // `or_insert_with` holds the shard lock for its entire call. Any + // occupied entry — InProgress, Awaiting, or Done — is left + // untouched, so at most one task is ever spawned per + // conversation id. self.title_tasks.entry(conversation.id).or_insert_with(|| { let (tx, rx) = oneshot::channel(); let handle = tokio::spawn(async move { diff --git a/crates/forge_app/src/mcp_executor.rs b/crates/forge_app/src/mcp_executor.rs index 21e3d024ba..0a2c80cc65 100644 --- a/crates/forge_app/src/mcp_executor.rs +++ b/crates/forge_app/src/mcp_executor.rs @@ -27,8 +27,9 @@ impl McpExecutor { pub async fn contains_tool(&self, tool_name: &ToolName) -> anyhow::Result { let mcp_servers = self.services.get_mcp_servers().await?; - // Convert Claude Code format (mcp__{server}__{tool}) to the internal legacy - // format (mcp_{server}_tool_{tool}) before checking, so both name styles match. + // Convert Claude Code format (mcp__{server}__{tool}) to the internal + // legacy format (mcp_{server}_tool_{tool}) before checking, so + // both name styles match. let legacy = tool_name.to_legacy_mcp_name(); let found = mcp_servers.get_servers().values().any(|tools| { tools diff --git a/crates/forge_app/src/operation.rs b/crates/forge_app/src/operation.rs index 1a88fce4f0..8a5bb64e91 100644 --- a/crates/forge_app/src/operation.rs +++ b/crates/forge_app/src/operation.rs @@ -358,7 +358,8 @@ impl ToolOperation { ); let display_lines = if truncated_output.start < truncated_output.end { - // Use 1-based indexing for display (humans count from 1) + // Use 1-based indexing for display (humans count from + // 1) format!("{}-{}", truncated_output.start + 1, truncated_output.end) } else { // No matches or empty result @@ -545,8 +546,8 @@ impl ToolOperation { forge_domain::ToolOutput::text(elm) } (Some(before), Some(after)) => { - // This diff is between modified state (before_undo) and snapshot - // state (after_undo) + // This diff is between modified state (before_undo) and + // snapshot state (after_undo) let diff = DiffFormat::format(before, after); let elm = Element::new("file_undo") @@ -1140,7 +1141,8 @@ mod tests { #[test] fn test_shell_output_both_stdout_stderr_truncation() { - // Create both stdout and stderr with more lines than the truncation limit + // Create both stdout and stderr with more lines than the truncation + // limit let mut stdout_lines = Vec::new(); for i in 1..=25 { stdout_lines.push(format!("stdout line {}", i)); @@ -1184,7 +1186,8 @@ mod tests { #[test] fn test_shell_output_exact_boundary_stdout() { - // Create stdout with exactly the truncation limit (prefix + suffix = 20 lines) + // Create stdout with exactly the truncation limit (prefix + suffix = 20 + // lines) let mut stdout_lines = Vec::new(); for i in 1..=20 { stdout_lines.push(format!("stdout line {}", i)); diff --git a/crates/forge_app/src/orch.rs b/crates/forge_app/src/orch.rs index e63ce75f1e..1a83f4a8ea 100644 --- a/crates/forge_app/src/orch.rs +++ b/crates/forge_app/src/orch.rs @@ -62,7 +62,8 @@ impl> Orc ) -> anyhow::Result> { let task_tool_name = ToolKind::Task.name(); - // Use a case-insensitive comparison since the model may send "Task" or "task". + // Use a case-insensitive comparison since the model may send "Task" or + // "task". let is_task = |tc: &ToolCallFull| { tc.name .as_str() @@ -76,8 +77,8 @@ impl> Orc |tc: &&ToolCallFull| tc.name.as_str().to_lowercase() == task_tool_name.as_str(); let (task_calls, other_calls): (Vec<_>, Vec<_>) = tool_calls.iter().partition(is_task_call); - // Execute task tool calls in parallel — mirrors how direct agent-as-tool calls - // work. + // Execute task tool calls in parallel — mirrors how direct + // agent-as-tool calls work. let task_results: Vec<(ToolCallFull, ToolResult)> = join_all( task_calls .iter() @@ -95,12 +96,13 @@ impl> Orc .map(|tool| &tool.name) .collect::>(); - // Process non-task tool calls sequentially (preserving UI notifier handshake - // and hooks). + // Process non-task tool calls sequentially (preserving UI notifier + // handshake and hooks). let mut other_results: Vec<(ToolCallFull, ToolResult)> = Vec::with_capacity(other_calls.len()); for tool_call in &other_calls { - // Send the start notification for system tools and not agent as a tool + // Send the start notification for system tools and not agent as a + // tool let is_system_tool = system_tools.contains(&tool_call.name); if is_system_tool { let notifier = Arc::new(Notify::new()); @@ -109,9 +111,10 @@ impl> Orc notifier: notifier.clone(), }) .await?; - // Wait for the UI to acknowledge it has rendered the tool header - // before we execute the tool. This prevents tool stdout from - // appearing before the tool name is printed. + // Wait for the UI to acknowledge it has rendered the tool + // header before we execute the tool. This + // prevents tool stdout from appearing before + // the tool name is printed. notifier.notified().await; } @@ -131,7 +134,8 @@ impl> Orc .call(&self.agent, tool_context, (*tool_call).clone()) .await; - // Fire the ToolcallEnd lifecycle event (fires on both success and failure) + // Fire the ToolcallEnd lifecycle event (fires on both success and + // failure) let toolcall_end_event = LifecycleEvent::ToolcallEnd(EventData::new( self.agent.clone(), self.agent.model.clone(), @@ -141,7 +145,8 @@ impl> Orc .handle(&toolcall_end_event, &mut self.conversation) .await?; - // Send the end notification for system tools and not agent as a tool + // Send the end notification for system tools and not agent as a + // tool if is_system_tool { self.send(ChatResponse::ToolCallEnd(tool_result.clone())) .await?; @@ -252,7 +257,8 @@ impl> Orc .handle(&start_event, &mut self.conversation) .await?; - // Signals that the loop should suspend (task may or may not be completed) + // Signals that the loop should suspend (task may or may not be + // completed) let mut should_yield = false; // Signals that the task is completed @@ -294,7 +300,8 @@ impl> Orc let model_id = model_id.clone(); move |error: &anyhow::Error, duration: Duration| { let root_cause = error.root_cause(); - // Log retry attempts - critical for debugging API failures + // Log retry attempts - critical for debugging API + // failures tracing::error!( agent_id = %agent_id, error = ?root_cause, @@ -319,8 +326,8 @@ impl> Orc .handle(&response_event, &mut self.conversation) .await?; - // Turn is completed, if finish_reason is 'stop'. Gemini models return stop as - // finish reason with tool calls. + // Turn is completed, if finish_reason is 'stop'. Gemini models + // return stop as finish reason with tool calls. is_complete = message.finish_reason == Some(FinishReason::Stop) && message.tool_calls.is_empty(); @@ -336,7 +343,8 @@ impl> Orc .execute_tool_calls(&message.tool_calls, &tool_context) .await?; - // Update context from conversation after response / tool-call hooks run + // Update context from conversation after response / tool-call hooks + // run if let Some(updated_context) = &self.conversation.context { context = updated_context.clone(); } @@ -346,7 +354,8 @@ impl> Orc for (_, result) in tool_call_records.iter_mut() { if result.is_error() { let attempts_left = self.error_tracker.remaining_attempts(&result.name); - // Add attempt information to the error message so the agent can reflect on it. + // Add attempt information to the error message so the agent + // can reflect on it. let context = serde_json::json!({ "attempts_left": attempts_left, "allowed_max_attempts": allowed_max_attempts, @@ -390,7 +399,8 @@ impl> Orc if !should_yield && let Some(max_request_allowed) = max_requests_per_turn { // Check if agent has reached the maximum request per turn limit if request_count >= max_request_allowed { - // Log warning - important for understanding conversation interruptions + // Log warning - important for understanding conversation + // interruptions warn!( agent_id = %self.agent.id, model_id = %model_id, diff --git a/crates/forge_app/src/orch_spec/orch_spec.rs b/crates/forge_app/src/orch_spec/orch_spec.rs index 4e5eaec96a..c1e9fc8da9 100644 --- a/crates/forge_app/src/orch_spec/orch_spec.rs +++ b/crates/forge_app/src/orch_spec/orch_spec.rs @@ -570,8 +570,8 @@ async fn test_is_complete_when_stop_with_no_tool_calls() { #[tokio::test] async fn test_not_complete_when_stop_with_tool_calls() { - // Test: is_complete = false when finish_reason is Stop BUT there are tool calls - // (Gemini models return stop as finish reason with tool calls) + // Test: is_complete = false when finish_reason is Stop BUT there are tool + // calls (Gemini models return stop as finish reason with tool calls) let tool_call = ToolCallFull::new("fs_read") .arguments(ToolCallArguments::from(json!({"path": "test.txt"}))); let tool_result = ToolResult::new("fs_read").output(Ok(ToolOutput::text("file content"))); @@ -603,9 +603,9 @@ async fn test_not_complete_when_stop_with_tool_calls() { #[tokio::test] async fn test_todo_enforcement_injects_reminder() { - // Test: When the orchestrator receives a Stop response but there are pending - // todos, the PendingTodosHandler hook should inject a formatted reminder - // message into the context listing all outstanding items. + // Test: When the orchestrator receives a Stop response but there are + // pending todos, the PendingTodosHandler hook should inject a formatted + // reminder message into the context listing all outstanding items. // NOTE: Since the End hook now adds reminders and triggers the outer loop // to continue, the orchestrator will loop until todos are completed. We // provide enough mock responses to verify the reminder is injected, and diff --git a/crates/forge_app/src/search_dedup.rs b/crates/forge_app/src/search_dedup.rs index fda84aef5d..48b9d814f7 100644 --- a/crates/forge_app/src/search_dedup.rs +++ b/crates/forge_app/src/search_dedup.rs @@ -60,7 +60,8 @@ impl Ord for Score { } } - // Compare in priority order: relevance → distance → similarity → query index + // Compare in priority order: relevance → distance → similarity → query + // index compare(self.relevance, other.relevance) // Higher relevance is better .or_else(|| compare(other.distance, self.distance)) // Lower distance is better (flipped) .unwrap_or_else(|| self.query_idx.cmp(&other.query_idx).reverse()) // Lower query index wins (first query wins) diff --git a/crates/forge_app/src/system_prompt.rs b/crates/forge_app/src/system_prompt.rs index a692f55bbf..e6e1cde77b 100644 --- a/crates/forge_app/src/system_prompt.rs +++ b/crates/forge_app/src/system_prompt.rs @@ -97,9 +97,10 @@ impl SystemPrompt { // Fetch extension statistics from git let extensions = self.fetch_extensions(self.max_extensions).await; - // Build tool_names map filtered to only the tools this agent actually has. - // This allows templates to use {{#if tool_names.task}} to conditionally - // render content based on whether the agent has access to a given tool. + // Build tool_names map filtered to only the tools this agent + // actually has. This allows templates to use {{#if + // tool_names.task}} to conditionally render content + // based on whether the agent has access to a given tool. let agent_tool_names: std::collections::HashSet = self .tool_definitions .iter() @@ -193,7 +194,8 @@ fn parse_extensions(extensions: &str, max_extensions: usize) -> Option::new(); all_files .iter() @@ -206,7 +208,8 @@ fn parse_extensions(extensions: &str, max_extensions: usize) -> Option = counts .into_iter() .map(|(extension, count)| { @@ -258,7 +261,8 @@ mod tests { let fixture = include_str!("fixtures/git_ls_files_mixed.txt"); let actual = parse_extensions(fixture, MAX_EXTENSIONS).unwrap(); - // 9 files: 4 rs, 2 md, 2 no-ext, 1 toml — sorted by count desc then alpha + // 9 files: 4 rs, 2 md, 2 no-ext, 1 toml — sorted by count desc then + // alpha let expected = Extension::new( vec![ ExtensionStat::new("rs", 4, "44"), @@ -277,9 +281,9 @@ mod tests { #[test] fn test_parse_extensions_truncates_to_max() { - // Real `git ls-files` output from this repo: 822 files, 19 distinct extensions. - // Top 15 are shown; the remaining 4 (html, jsonl, lock, proto — 1 each) are - // rolled up. + // Real `git ls-files` output from this repo: 822 files, 19 distinct + // extensions. Top 15 are shown; the remaining 4 (html, jsonl, + // lock, proto — 1 each) are rolled up. let fixture = include_str!("fixtures/git_ls_files_many_extensions.txt"); let actual = parse_extensions(fixture, MAX_EXTENSIONS).unwrap(); diff --git a/crates/forge_app/src/template_engine.rs b/crates/forge_app/src/template_engine.rs index bb7f41c336..4eae935155 100644 --- a/crates/forge_app/src/template_engine.rs +++ b/crates/forge_app/src/template_engine.rs @@ -88,7 +88,8 @@ fn create_handlebar() -> Handlebars<'static> { false }; - // Write "true" or empty string for handlebars to interpret as boolean + // Write "true" or empty string for handlebars to interpret as + // boolean if contains { out.write("true")?; } diff --git a/crates/forge_app/src/terminal_context.rs b/crates/forge_app/src/terminal_context.rs index 0079342938..c7edd34d32 100644 --- a/crates/forge_app/src/terminal_context.rs +++ b/crates/forge_app/src/terminal_context.rs @@ -69,9 +69,9 @@ impl> TerminalContextSer .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. + // 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))) @@ -260,7 +260,8 @@ mod tests { #[test] fn test_commands_sorted_by_timestamp_oldest_first() { - // Supply commands in reverse-timestamp order to confirm sorting is applied. + // Supply commands in reverse-timestamp order to confirm sorting is + // applied. let sep = ENV_LIST_SEPARATOR; let fixture = TerminalContextService::new(MockInfra::new(&[ ( diff --git a/crates/forge_app/src/title_generator.rs b/crates/forge_app/src/title_generator.rs index bd95f03c05..be3e49dd97 100644 --- a/crates/forge_app/src/title_generator.rs +++ b/crates/forge_app/src/title_generator.rs @@ -80,12 +80,13 @@ impl TitleGenerator { .await?; let ChatCompletionMessageFull { content, .. } = stream.into_full(false).await?; - // Parse the response - try JSON first (structured output), fallback to plain - // text + // Parse the response - try JSON first (structured output), fallback to + // plain text match serde_json::from_str::(&content) { Ok(response) => Ok(Some(response.title)), Err(_) => { - // Fallback: Some providers don't support structured output, treat as plain text + // Fallback: Some providers don't support structured output, + // treat as plain text Ok(Some(content.trim().to_string())) } } diff --git a/crates/forge_app/src/tool_registry.rs b/crates/forge_app/src/tool_registry.rs index dbfff3da06..03cea01f9a 100644 --- a/crates/forge_app/src/tool_registry.rs +++ b/crates/forge_app/src/tool_registry.rs @@ -71,7 +71,8 @@ impl> ToolReg if let Some(operation) = operation { let decision = self.services.check_operation_permission(&operation).await?; - // Send custom policy message to the user when a policy file was created + // Send custom policy message to the user when a policy file was + // created if let Some(policy_path) = decision.path { use forge_domain::TitleFormat; @@ -137,8 +138,9 @@ impl> ToolReg context.send(content).await?; } - // Check permissions before executing the tool (only in restricted mode) - // This is done BEFORE the timeout to ensure permissions are never timed out + // Check permissions before executing the tool (only in restricted + // mode) This is done BEFORE the timeout to ensure + // permissions are never timed out let is_restricted = self.services.get_config()?.restricted; if is_restricted && self.check_tool_permission(&tool_input, context).await? { // Send formatted output message for policy denial @@ -153,7 +155,8 @@ impl> ToolReg } // Validate tool modality support before execution - // Only resolve the current model when modality validation is needed. + // Only resolve the current model when modality validation is + // needed. if matches!(&tool_input, ToolCatalog::Read(input) if Self::has_image_extension(&input.file_path)) { let model = self.get_current_model().await; @@ -259,7 +262,8 @@ impl> ToolReg // Build TemplateConfig from ForgeConfig for tool description templates let config = self.services.get_config()?; - // Filter out research subagents from task tool description when disabled + // Filter out research subagents from task tool description when + // disabled if !config.research_subagent { agents.retain(|agent| { let id = agent.id.as_str(); @@ -306,7 +310,8 @@ impl ToolRegistry { // Build tool_names map from all available tools let tool_names: Map = ToolCatalog::iter() .filter(|tool| { - // Only include tools that are supported (filter sem_search if not supported) + // Only include tools that are supported (filter sem_search if + // not supported) if matches!(tool, ToolCatalog::SemSearch(_)) { sem_search_supported } else { @@ -603,9 +608,9 @@ mod tests { let actual = ToolRegistry::<()>::validate_tool_call(&fixture, &ToolName::new("tool_[special]")); - // The glob pattern "tool_[special]" will match "tool_s", "tool_p", etc., not - // the literal string So this test verifies that exact matching doesn't - // work when the pattern is a valid glob + // The glob pattern "tool_[special]" will match "tool_s", "tool_p", + // etc., not the literal string So this test verifies that exact + // matching doesn't work when the pattern is a valid glob assert!(actual.is_err()); } @@ -1096,7 +1101,8 @@ fn test_all_rendered_tool_descriptions() { } // Snapshot all rendered tool descriptions for visual verification - // This will fail if a tool is renamed and descriptions reference the old name + // This will fail if a tool is renamed and descriptions reference the old + // name let all_descriptions: Vec<_> = tools .iter() .map(|t| format!("### {}\n\n{}\n", t.name, t.description)) diff --git a/crates/forge_app/src/tool_resolver.rs b/crates/forge_app/src/tool_resolver.rs index a3b768a0ac..1da2dfb8e5 100644 --- a/crates/forge_app/src/tool_resolver.rs +++ b/crates/forge_app/src/tool_resolver.rs @@ -333,8 +333,8 @@ mod tests { let actual = tool_resolver.resolve(&fixture); // fs_write matches fs_* at pos 0 - // fs_read has exact match at pos 1 (takes precedence over pattern matches) - // So order is: fs_write (pos 0), fs_read (pos 1) + // fs_read has exact match at pos 1 (takes precedence over pattern + // matches) So order is: fs_write (pos 0), fs_read (pos 1) let expected = vec![ &tool_resolver.all_tool_definitions[1], // fs_write &tool_resolver.all_tool_definitions[0], // fs_read @@ -392,8 +392,8 @@ mod tests { .tools(vec![ToolName::new("read"), ToolName::new("search")]); let actual = tool_resolver.resolve(&fixture); - // Tools are ordered as specified in the tools list: read, then search (-> - // fs_search) + // Tools are ordered as specified in the tools list: read, then search + // (-> fs_search) let expected = vec![ &tool_resolver.all_tool_definitions[0], // read &tool_resolver.all_tool_definitions[1], // fs_search (from "search" alias) diff --git a/crates/forge_app/src/transformers/model_specific_reasoning.rs b/crates/forge_app/src/transformers/model_specific_reasoning.rs index 43f25c7539..2f59ae6c78 100644 --- a/crates/forge_app/src/transformers/model_specific_reasoning.rs +++ b/crates/forge_app/src/transformers/model_specific_reasoning.rs @@ -35,9 +35,10 @@ impl ModelSpecificReasoning { || id.contains("mythos") || id.contains("fable") { - // Opus 5 and Opus 4.8 share Opus 4.7's API contract: adaptive thinking only - // (legacy `budget_tokens` returns 400) and non-default sampling - // params (`temperature`/`top_p`/`top_k`) return 400. + // Opus 5 and Opus 4.8 share Opus 4.7's API contract: adaptive + // thinking only (legacy `budget_tokens` returns 400) + // and non-default sampling params (`temperature`/ + // `top_p`/`top_k`) return 400. AnthropicModelFamily::AdaptiveOnly } else if id.contains("opus-4-6") || id.contains("46-opus") @@ -415,8 +416,8 @@ mod tests { #[test] fn test_legacy_no_effort_drops_effort_for_all_pre_4_5_ids() { // All pre-Opus-4.5 Claude ids (plus the newer non-effort family members - // Sonnet 4.5 and Haiku 4.5) should land in LegacyNoEffort and have their - // effort stripped. + // Sonnet 4.5 and Haiku 4.5) should land in LegacyNoEffort and have + // their effort stripped. for model in [ "claude-sonnet-4-5-20250929", "claude-haiku-4-5-20251001", diff --git a/crates/forge_app/src/transformers/strip_working_dir.rs b/crates/forge_app/src/transformers/strip_working_dir.rs index dcb6911251..c3bf90138c 100644 --- a/crates/forge_app/src/transformers/strip_working_dir.rs +++ b/crates/forge_app/src/transformers/strip_working_dir.rs @@ -515,8 +515,9 @@ mod tests { #[test] fn test_windows_case_sensitivity() { - // On Windows, paths are case-insensitive, but we preserve the original case - // when stripping. This test verifies case-sensitive matching behavior. + // On Windows, paths are case-insensitive, but we preserve the original + // case when stripping. This test verifies case-sensitive + // matching behavior. let fixture = ContextSummary::new(vec![SummaryBlock::new( Role::Assistant, vec![ @@ -526,9 +527,9 @@ mod tests { )]); let actual = StripWorkingDir::new(r"C:\Users\User\Project").transform(fixture); - // On Windows: case-insensitive matching, first path strips, second doesn't - // On Unix: case-sensitive matching, neither path strips (Windows paths not - // recognized) + // On Windows: case-insensitive matching, first path strips, second + // doesn't On Unix: case-sensitive matching, neither path strips + // (Windows paths not recognized) #[cfg(windows)] let expected = ContextSummary::new(vec![SummaryBlock::new( Role::Assistant, diff --git a/crates/forge_app/src/transformers/trim_context_summary.rs b/crates/forge_app/src/transformers/trim_context_summary.rs index 9207c77b5b..1014127b8a 100644 --- a/crates/forge_app/src/transformers/trim_context_summary.rs +++ b/crates/forge_app/src/transformers/trim_context_summary.rs @@ -394,8 +394,8 @@ mod tests { )]); let actual = TrimContextSummary.transform(fixture); - // Shell commands break the deduplication chain, so both reads of /test.rs are - // preserved + // Shell commands break the deduplication chain, so both reads of + // /test.rs are preserved let expected = ContextSummary::new(vec![SummaryBlock::new( Role::Assistant, vec![ @@ -424,8 +424,9 @@ mod tests { )]); let actual = TrimContextSummary.transform(fixture); - // Shell commands break the deduplication chain - all reads are preserved - // because shell commands are interspersed between them + // Shell commands break the deduplication chain - all reads are + // preserved because shell commands are interspersed between + // them let expected = ContextSummary::new(vec![SummaryBlock::new( Role::Assistant, vec![ diff --git a/crates/forge_app/src/user_prompt.rs b/crates/forge_app/src/user_prompt.rs index b076c58933..886e58bf2d 100644 --- a/crates/forge_app/src/user_prompt.rs +++ b/crates/forge_app/src/user_prompt.rs @@ -155,7 +155,8 @@ impl 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 + // 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 { @@ -233,10 +234,12 @@ impl // Track file attachments as read operations in metrics let mut metrics = conversation.metrics.clone(); for attachment in &attachments { - // Only track file content attachments (not images or directory listings). - // Use the raw content_hash (computed before line-numbering) so that the - // external-change detector, which hashes the raw file on disk, sees a - // matching hash and does not raise a false "modified externally" warning. + // Only track file content attachments (not images or directory + // listings). Use the raw content_hash (computed before + // line-numbering) so that the external-change detector, + // which hashes the raw file on disk, sees a + // matching hash and does not raise a false "modified externally" + // warning. if let AttachmentContent::FileContent { info, .. } = &attachment.content { metrics = metrics.insert( attachment.path.clone(), @@ -514,7 +517,8 @@ mod tests { "file2.rs should have content hash" ); - // Verify both files are in files_accessed (since they are Read operations) + // Verify both files are in files_accessed (since they are Read + // operations) assert!( actual.metrics.files_accessed.contains("/test/file1.rs"), "file1.rs should be in files_accessed" @@ -563,8 +567,8 @@ mod tests { let agent = fixture_agent_without_user_prompt(); let event = Event::new("Continue working"); - // Create a conversation with existing context (simulating resume) and todos - // stored in metrics + // Create a conversation with existing context (simulating resume) and + // todos stored in metrics let conversation = Conversation::new(ConversationId::generate()) .context( Context::default() @@ -587,7 +591,8 @@ mod tests { // Execute let actual = generator.add_user_prompt(conversation).await.unwrap(); - // Assert - Should have system, previous user, new user message, and todo list + // Assert - Should have system, previous user, new user message, and + // todo list let messages = actual.context.unwrap().messages; assert_eq!(messages.len(), 4, "Should have 4 messages"); diff --git a/crates/forge_app/src/utils.rs b/crates/forge_app/src/utils.rs index 349157d793..aeb3a1f086 100644 --- a/crates/forge_app/src/utils.rs +++ b/crates/forge_app/src/utils.rs @@ -455,9 +455,9 @@ pub fn enforce_strict_schema(schema: &mut serde_json::Value, strict_mode: bool) // supported OpenAI fields; Forge keeps raw JSON schemas, so we // explicitly remove unsupported validation/meta keywords here. normalize_openai_schema_subset_keywords(map); - // Convert oneOf to anyOf because the Responses API rejects oneOf - // in tool parameter schemas while accepting equivalent anyOf - // unions. + // Convert oneOf to anyOf because the Responses API rejects + // oneOf in tool parameter schemas while + // accepting equivalent anyOf unions. normalize_one_of_keyword(map, strict_mode); } @@ -465,8 +465,9 @@ pub fn enforce_strict_schema(schema: &mut serde_json::Value, strict_mode: bool) let is_object = is_object_schema(map); - // If this looks like an object schema but has no explicit type, add it - // OpenAI requires all schemas to have a type when they represent objects + // If this looks like an object schema but has no explicit type, add + // it OpenAI requires all schemas to have a type when + // they represent objects if is_object && !map.contains_key("type") { map.insert( "type".to_string(), @@ -510,10 +511,12 @@ pub fn enforce_strict_schema(schema: &mut serde_json::Value, strict_mode: bool) && !map.contains_key("anyOf") && !map.contains_key("allOf") { - // In strict mode, OpenAI/Codex requires all property schemas to have a - // 'type' key. External MCP tool schemas may define properties with only a - // description and no type. Default such typeless leaf schemas to "string" - // so the request is not rejected with "schema must have a 'type' key". + // In strict mode, OpenAI/Codex requires all property schemas to + // have a 'type' key. External MCP tool schemas + // may define properties with only a description + // and no type. Default such typeless leaf schemas to "string" + // so the request is not rejected with "schema must have a + // 'type' key". map.insert( "type".to_string(), serde_json::Value::String("string".to_string()), @@ -639,8 +642,8 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { // OpenAPI 3.0 doesn't support type arrays, so we convert them: // - ["string", "null"] -> type: "string", nullable: true // - ["string", "number"] -> anyOf: [{type: string}, {type: number}] - // - ["string", "number", "null"] -> anyOf: [{type: string}, {type: number}], - // nullable: true + // - ["string", "number", "null"] -> anyOf: [{type: string}, {type: + // number}], nullable: true if map.contains_key("type") && map["type"].is_array() { let types = map.remove("type").unwrap(); if let serde_json::Value::Array(type_arr) = types { @@ -655,8 +658,8 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { serde_json::Value::String("null".to_string()), ); } else if non_null_types.len() == 1 { - // Single non-null type: ["string", "null"] -> type: "string", nullable: - // true + // Single non-null type: ["string", "null"] -> type: + // "string", nullable: true map.insert( "type".to_string(), non_null_types.into_iter().next().unwrap(), @@ -679,10 +682,10 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { } // Handle anyOf with null type — elevate null to nullable. - // { anyOf: [{type: string, ...}, {type: null}] } -> { type: string, nullable: - // true, ... } { anyOf: [{type: string, ...}, {type: number, ...}, - // {type: null}] } -> { anyOf: [{type: string}, {type: number}], nullable: true - // } + // { anyOf: [{type: string, ...}, {type: null}] } -> { type: string, + // nullable: true, ... } { anyOf: [{type: string, ...}, + // {type: number, ...}, {type: null}] } -> { anyOf: + // [{type: string}, {type: number}], nullable: true } if let Some(serde_json::Value::Array(any_of)) = map.remove("anyOf") { let (null_schemas, non_null_schemas): (Vec<_>, Vec<_>) = any_of.into_iter().partition(|s| { @@ -692,11 +695,13 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { }); if !null_schemas.is_empty() && non_null_schemas.len() == 1 { - // Single non-null branch with nullable: merge into this schema + // Single non-null branch with nullable: merge into this + // schema let mut merged = non_null_schemas.into_iter().next().unwrap(); if let serde_json::Value::Object(merged_map) = &mut merged { // Copy current schema's keys into the merged branch - // (anyOf was already removed, so we copy everything else) + // (anyOf was already removed, so we copy everything + // else) let current_keys: Vec<(String, serde_json::Value)> = map.iter().map(|(k, v)| (k.clone(), v.clone())).collect(); for (key, value) in current_keys { @@ -723,9 +728,10 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { } } - // Convert integer/number enum values to strings (Gemini rejects integer - // enums). Only change the type when there's an enum — a bare integer/number - // type without enum is valid for Gemini. + // Convert integer/number enum values to strings (Gemini rejects + // integer enums). Only change the type when there's an + // enum — a bare integer/number type without enum is + // valid for Gemini. let has_numeric_type_with_enum = map .get("type") .and_then(|v| v.as_str()) @@ -766,8 +772,9 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { map.insert("items".to_string(), serde_json::json!({ "type": "string" })); } Some(serde_json::Value::Object(items_map)) => { - // Items exists but may be empty — ensure it has at least a - // type if it has no schema-defining keywords + // Items exists but may be empty — ensure it has at + // least a type if it has no + // schema-defining keywords let has_schema_intent = items_map.contains_key("type") || items_map.contains_key("$ref") || items_map.contains_key("enum") @@ -808,7 +815,8 @@ pub fn sanitize_gemini_schema(schema: &mut serde_json::Value) { map.remove("required"); } - // Filter required array to only include fields present in properties + // Filter required array to only include fields present in + // properties let property_keys: Option> = map .get("properties") .and_then(|v| v.as_object()) @@ -994,9 +1002,10 @@ mod tests { #[test] fn test_typeless_property_gets_string_type_in_strict_mode() { - // MCP tool schemas from external servers (e.g. Affine) may define properties - // with only a description and no type key. The OpenAI/Codex endpoint rejects - // such schemas with "schema must have a 'type' key". This test verifies that + // MCP tool schemas from external servers (e.g. Affine) may define + // properties with only a description and no type key. The + // OpenAI/Codex endpoint rejects such schemas with "schema must + // have a 'type' key". This test verifies that // enforce_strict_schema defaults typeless leaf properties to "string". let mut schema = json!({ "type": "object", @@ -1363,7 +1372,8 @@ mod tests { #[test] fn test_notion_mcp_create_comment_schema() { - // Simulates the actual Notion MCP create_comment schema that was failing + // Simulates the actual Notion MCP create_comment schema that was + // failing let mut schema = json!({ "type": "object", "properties": { @@ -1419,15 +1429,16 @@ mod tests { assert_eq!(schema["type"], "object"); assert_eq!(schema["properties"]["rich_text"]["type"], "array"); - // 2. Check that the anyOf items have proper types and additionalProperties: - // false + // 2. Check that the anyOf items have proper types and + // additionalProperties: false let any_of = schema["properties"]["rich_text"]["items"]["anyOf"] .as_array() .unwrap(); for branch in any_of { assert_eq!(branch["type"], "object"); assert_eq!(branch["additionalProperties"], false); - // All nested object properties should also have type and additionalProperties + // All nested object properties should also have type and + // additionalProperties if let Some(props) = branch["properties"].as_object() { for (_, prop_schema) in props { if let Some(obj) = prop_schema.as_object() @@ -1457,7 +1468,8 @@ mod tests { #[test] fn test_property_names_is_removed_in_strict_mode() { // This test ensures we don't regress on propertyNames removal - // propertyNames is a JSON Schema keyword that OpenAI/Codex doesn't support + // propertyNames is a JSON Schema keyword that OpenAI/Codex doesn't + // support let mut schema = json!({ "type": "object", "properties": { @@ -2091,7 +2103,8 @@ mod tests { #[test] fn test_gemini_converts_multi_type_array_with_null() { - // Should become: anyOf: [{type: string}, {type: number}], nullable: true + // Should become: anyOf: [{type: string}, {type: number}], nullable: + // true let mut schema = json!({ "type": "object", "properties": { diff --git a/crates/forge_display/src/code.rs b/crates/forge_display/src/code.rs index e754c9d0cb..52fbcc7040 100644 --- a/crates/forge_display/src/code.rs +++ b/crates/forge_display/src/code.rs @@ -23,7 +23,8 @@ pub struct SyntaxHighlighter { impl Default for SyntaxHighlighter { fn default() -> Self { - // Use two-face's extended syntax set which includes TOML, Rust, Python, etc. + // Use two-face's extended syntax set which includes TOML, Rust, Python, + // etc. Self { syntax_set: Arc::new(two_face::syntax::extra_newlines()), theme_set: Arc::new(two_face::theme::extra().into()), @@ -136,7 +137,8 @@ impl CodeBlockParser { fn detect_code_fence(line: &str) -> Option { let trimmed = line.trim_start(); if let Some(stripped) = trimmed.strip_prefix("```") { - // Extract language tag (everything after ``` until whitespace or end) + // Extract language tag (everything after ``` until whitespace or + // end) let lang = stripped.split_whitespace().next().unwrap_or(""); Some(lang.to_string()) } else { diff --git a/crates/forge_display/src/diff.rs b/crates/forge_display/src/diff.rs index 338e3a0266..d6686fc509 100644 --- a/crates/forge_display/src/diff.rs +++ b/crates/forge_display/src/diff.rs @@ -61,8 +61,8 @@ impl DiffFormat { return DiffResult { result: output, lines_added, lines_removed }; } - // First pass: Calculate dynamic width based on max line numbers in actual - // changes + // First pass: Calculate dynamic width based on max line numbers in + // actual changes let mut max_line_number = 0; for group in &ops { for op in group { @@ -201,8 +201,8 @@ mod tests { let diff = DiffFormat::format(&old, &new); let clean_diff = strip_ansi_codes(diff.diff()); - // Diff only shows lines 3-8 (context), so width should be 1 (for single digit - // numbers) NOT 4 (which would be needed for line 1000) + // Diff only shows lines 3-8 (context), so width should be 1 (for single + // digit numbers) NOT 4 (which would be needed for line 1000) assert!(clean_diff.contains("3 3 | line 3")); assert!(clean_diff.contains("5 |-line 5")); assert_eq!(diff.lines_added(), 1); diff --git a/crates/forge_display/src/grep.rs b/crates/forge_display/src/grep.rs index f0398f7ffc..5b9c3a4f01 100644 --- a/crates/forge_display/src/grep.rs +++ b/crates/forge_display/src/grep.rs @@ -141,7 +141,8 @@ impl GrepFormat { .iter() .any(|line| ParsedLine::parse(line).is_some()); - // If no valid grep format entries found, treat all lines as raw file paths + // If no valid grep format entries found, treat all lines as raw file + // paths if !has_valid_entries { return self.format_raw_paths(); } diff --git a/crates/forge_domain/src/agent.rs b/crates/forge_domain/src/agent.rs index ace8bfdfc0..072117037e 100644 --- a/crates/forge_domain/src/agent.rs +++ b/crates/forge_domain/src/agent.rs @@ -335,8 +335,9 @@ mod tests { #[test] fn test_cap_compact_token_threshold_caps_to_safe_margin_when_within_context_window() { - // With the fix, thresholds are capped to 70% of context window for safety - // even when they're technically "within" the context window + // With the fix, thresholds are capped to 70% of context window for + // safety even when they're technically "within" the context + // window let fixture = Agent::new( AgentId::new("test"), ProviderId::OPENAI, @@ -392,8 +393,9 @@ mod tests { #[test] fn test_cap_compact_token_threshold_uses_default_when_selected_model_is_missing() { - // With the fix, even without model info, we set a safe default threshold - // based on a default context window of 128K (70% = 89.6K) + // With the fix, even without model info, we set a safe default + // threshold based on a default context window of 128K (70% = + // 89.6K) let fixture = Agent::new( AgentId::new("test"), ProviderId::OPENAI, @@ -427,8 +429,9 @@ mod tests { let actual = fixture.compaction_threshold(Some(&selected_model)); - // EXPECTED: Should set default threshold to 70% of context window (128000 * 0.7 - // = 89600) ACTUAL BUG: Returns early with token_threshold still as None + // EXPECTED: Should set default threshold to 70% of context window + // (128000 * 0.7 = 89600) ACTUAL BUG: Returns early with + // token_threshold still as None let expected_threshold = Some(89_600); assert_eq!( actual.compact.token_threshold, expected_threshold, @@ -457,16 +460,18 @@ mod tests { let actual = fixture.compaction_threshold(Some(&selected_model)); // The current logic keeps 100000 because 100000 < 128000 - // But this leaves only 28000 tokens of headroom for tool outputs and new - // messages When context is at 105000 tokens, compaction won't trigger - // (below 100K threshold) But adding tool outputs (5000 tokens) + new - // user message (2000 tokens) = 112000 API request with 112000 tokens - // succeeds Next turn: context at 112000, still below 100K threshold - // Adding more tool outputs: 112000 + 20000 = 132000 > 128000 limit → + // But this leaves only 28000 tokens of headroom for tool outputs and + // new messages When context is at 105000 tokens, compaction + // won't trigger (below 100K threshold) But adding tool outputs + // (5000 tokens) + new user message (2000 tokens) = 112000 API + // request with 112000 tokens succeeds Next turn: context at + // 112000, still below 100K threshold Adding more tool outputs: + // 112000 + 20000 = 132000 > 128000 limit → // context_length_exceeded! - // EXPECTED: Threshold should be capped to provide safety margin (70% = 89600) - // ACTUAL BUG: Threshold stays at 100000, causing eventual overflow + // EXPECTED: Threshold should be capped to provide safety margin (70% = + // 89600) ACTUAL BUG: Threshold stays at 100000, causing + // eventual overflow let expected_safe_threshold = Some(89_600); assert_eq!( actual.compact.token_threshold, expected_safe_threshold, @@ -493,9 +498,10 @@ mod tests { let actual = fixture.compaction_threshold(Some(&selected_model)); - // EXPECTED: Should set a reasonable default threshold (e.g., 64000 for 128K - // default window) or at least set SOME threshold to prevent unbounded - // growth ACTUAL BUG: Returns early with token_threshold still as None + // EXPECTED: Should set a reasonable default threshold (e.g., 64000 for + // 128K default window) or at least set SOME threshold to + // prevent unbounded growth ACTUAL BUG: Returns early with + // token_threshold still as None assert!( actual.compact.token_threshold.is_some(), "BUG: compaction_threshold should set a default threshold even when model context_length is unknown. \ diff --git a/crates/forge_domain/src/compact/compact_config.rs b/crates/forge_domain/src/compact/compact_config.rs index 4b406509ec..58925e7ea7 100644 --- a/crates/forge_domain/src/compact/compact_config.rs +++ b/crates/forge_domain/src/compact/compact_config.rs @@ -140,7 +140,8 @@ impl Compact { fn should_compact_due_to_tokens(&self, token_count: usize) -> bool { if let Some(token_threshold) = self.token_threshold { debug!(tokens = ?token_count, "Token count"); - // use provided prompt_tokens if available, otherwise estimate token count + // use provided prompt_tokens if available, otherwise estimate token + // count token_count >= token_threshold } else { false diff --git a/crates/forge_domain/src/compact/strategy.rs b/crates/forge_domain/src/compact/strategy.rs index 01f6fade6e..e243307d82 100644 --- a/crates/forge_domain/src/compact/strategy.rs +++ b/crates/forge_domain/src/compact/strategy.rs @@ -103,8 +103,8 @@ fn find_sequence_preserving_last_n( } // Calculate the end index based on preservation window - // If we need to preserve all or more messages than we have, there's nothing to - // compact + // If we need to preserve all or more messages than we have, there's nothing + // to compact if max_retention >= length { return None; } @@ -119,8 +119,9 @@ fn find_sequence_preserving_last_n( // Don't break between a tool call and its result if messages.get(end).is_some_and(|msg| msg.has_tool_call()) { - // If the last message has a tool call, adjust end to include the tool result - // This means either not compacting at all, or reducing the end by 1 + // If the last message has a tool call, adjust end to include the tool + // result This means either not compacting at all, or reducing + // the end by 1 if end == start { // If start == end and it has a tool call, don't compact return None; @@ -135,8 +136,8 @@ fn find_sequence_preserving_last_n( .get(end.saturating_add(1)) .is_some_and(|msg| msg.has_tool_result()) { - // If the last message is a tool result and the next one is also a tool result, - // we need to adjust the end. + // If the last message is a tool result and the next one is also a tool + // result, we need to adjust the end. while end >= start && messages.get(end).is_some_and(|msg| msg.has_tool_result()) { end = end.saturating_sub(1); } @@ -342,13 +343,16 @@ mod tests { let fixture = context_from_pattern("sua"); // Test Percentage strategy conversion - // Context: System (3 tokens), User (3 tokens), Assistant (3 tokens) = 9 total - // tokens Eviction budget: 40% of 9 = 3.6 → 4 tokens (rounded up) - // Strategy skips system messages, so calculation for non-system messages: - // - User message (index 1): 3 tokens → budget: 4 - 3 = 1 token remaining - // - Assistant message (index 2): 3 tokens → budget: 1 - 3 = 0 (saturating_sub) - // Result: Eviction budget exhausted at index 2 (Assistant), so to_fixed returns - // 2 + // Context: System (3 tokens), User (3 tokens), Assistant (3 tokens) = 9 + // total tokens Eviction budget: 40% of 9 = 3.6 → 4 tokens + // (rounded up) Strategy skips system messages, so calculation + // for non-system messages: + // - User message (index 1): 3 tokens → budget: 4 - 3 = 1 token + // remaining + // - Assistant message (index 2): 3 tokens → budget: 1 - 3 = 0 + // (saturating_sub) + // Result: Eviction budget exhausted at index 2 (Assistant), so to_fixed + // returns 2 let percentage_strategy = CompactionStrategy::evict(0.4); let actual = percentage_strategy.to_fixed(&fixture); let expected = 2; @@ -362,8 +366,8 @@ mod tests { // Test invalid percentage (gets clamped to 1.0 = 100%) // With 100% eviction budget (9 tokens), we can evict all messages - // With 9 tokens budget, all 3 messages (3+3+3) exhaust the budget at message - // index 2 + // With 9 tokens budget, all 3 messages (3+3+3) exhaust the budget at + // message index 2 let invalid_strategy = CompactionStrategy::evict(1.5); let actual = invalid_strategy.to_fixed(&fixture); let expected = 2; // Returns index 2 (last message) when all messages fit in budget @@ -394,8 +398,8 @@ mod tests { let percentage_strategy = CompactionStrategy::evict(0.4); percentage_strategy.to_fixed(&fixture); - // Use fixed window strategy - preserve last 1 message, starting from first - // assistant + // Use fixed window strategy - preserve last 1 message, starting from + // first assistant let preserve_strategy = CompactionStrategy::retain(1); let actual_sequence = preserve_strategy.eviction_range(&fixture); let expected = Some((1, 2)); // Start from first assistant at index 1 diff --git a/crates/forge_domain/src/compact/summary.rs b/crates/forge_domain/src/compact/summary.rs index 3416dfdba8..bb040f49ed 100644 --- a/crates/forge_domain/src/compact/summary.rs +++ b/crates/forge_domain/src/compact/summary.rs @@ -231,7 +231,8 @@ impl From<&Context> for ContextSummary { } if current_role != text_msg.role { - // Only push if buffer is not empty (avoid empty System role at start) + // Only push if buffer is not empty (avoid empty System + // role at start) if !buffer.is_empty() { messages.push(SummaryBlock { role: current_role, diff --git a/crates/forge_domain/src/context.rs b/crates/forge_domain/src/context.rs index c2f0f30fde..80d074973a 100644 --- a/crates/forge_domain/src/context.rs +++ b/crates/forge_domain/src/context.rs @@ -570,10 +570,11 @@ impl Context { tool_records: Vec<(ToolCallFull, ToolResult)>, phase: Option, ) -> Self { - // Convert flat reasoning string to reasoning_details only when no structured - // reasoning_details are present. When reasoning_details already exists it - // already contains the text (with its cryptographic signature), so adding - // another entry from the raw `reasoning` string would produce a duplicate + // Convert flat reasoning string to reasoning_details only when no + // structured reasoning_details are present. When + // reasoning_details already exists it already contains the text + // (with its cryptographic signature), so adding another entry + // from the raw `reasoning` string would produce a duplicate // thinking block with a null signature, which Anthropic rejects. let merged_reasoning_details = match (reasoning, reasoning_details) { (_, Some(details)) => Some(details), @@ -645,7 +646,8 @@ impl Context { return false; } - // When enabled parameter is defined then return it's value directly. + // When enabled parameter is defined then return it's value + // directly. if reasoning.enabled.is_some() { return reasoning.enabled.unwrap_or_default(); } @@ -867,7 +869,8 @@ mod tests { let token_count = estimate_token_count(context.to_text().len()); // Validate the token count is reasonable - // The exact value will depend on the implementation of estimate_token_count + // The exact value will depend on the implementation of + // estimate_token_count assert!(token_count > 0, "Token count should be greater than 0"); } @@ -1065,8 +1068,8 @@ mod tests { let fixture = Context::default().messages(vec![wrapper]); assert_eq!(fixture.token_count(), TokenCount::Actual(80)); - // case 4: context with messages - since total_tokens are not present return - // estimate + // case 4: context with messages - since total_tokens are not present + // return estimate let fixture = Context::default() .add_message(ContextMessage::user("Hello", None)) .add_message(ContextMessage::assistant("Hi there!", None, None, None)) @@ -1106,8 +1109,8 @@ mod tests { let actual = fixture.token_count(); - // Expected: Should use the LAST message's usage (300), not the first (100) or - // second (200) + // Expected: Should use the LAST message's usage (300), not the first + // (100) or second (200) let expected = TokenCount::Actual(300); assert_eq!(actual, expected); @@ -1200,8 +1203,9 @@ mod tests { #[test] fn test_context_is_reasoning_not_supported_when_effort_is_none() { - // `Effort::None` is documented as "skips the thinking step entirely" and - // must act as an explicit opt-out regardless of other fields. + // `Effort::None` is documented as "skips the thinking step entirely" + // and must act as an explicit opt-out regardless of other + // fields. let fixture = Context::default().reasoning(crate::ReasoningConfig { effort: Some(crate::Effort::None), ..Default::default() @@ -1389,8 +1393,8 @@ mod tests { }, ]); - // Test total messages (6 messages: 1 system + 2 user + 2 assistant + 2 tool - // results) + // Test total messages (6 messages: 1 system + 2 user + 2 assistant + 2 + // tool results) assert_eq!(fixture.total_messages(), 7); // Test user message count @@ -1405,8 +1409,9 @@ mod tests { #[test] fn test_directory_listing_sorted_dirs_first() { - // Create entries already sorted (as they would come from attachment service) - // Directories first, then files, all sorted alphabetically + // Create entries already sorted (as they would come from attachment + // service) Directories first, then files, all sorted + // alphabetically let fixture_attachments = vec![Attachment { path: "/test/root".to_string(), content: AttachmentContent::DirectoryListing { @@ -1500,9 +1505,10 @@ mod tests { ContextMessage::assistant("Let me help", None, None, Some(fixture_tool_calls)); let actual = fixture.token_count_approx(); // Content: "Let me help" = 11 chars - // Tool call 1: "fs_search" (9 chars) + {"query":"test"} (16 chars) = 25 chars - // Tool call 2: "calculate" (9 chars) + {"expression":"2+2"} (20 chars) = 29 - // chars Total: 11 + 25 + 29 = 65 chars / 4 = 17 tokens + // Tool call 1: "fs_search" (9 chars) + {"query":"test"} (16 chars) = 25 + // chars Tool call 2: "calculate" (9 chars) + + // {"expression":"2+2"} (20 chars) = 29 chars Total: 11 + 25 + + // 29 = 65 chars / 4 = 17 tokens let expected = 17; assert_eq!(actual, expected); } @@ -1665,8 +1671,8 @@ mod tests { #[test] fn test_has_model_changed_ignores_user_messages() { - // User messages have model tracking too, but we should only check assistant - // messages + // User messages have model tracking too, but we should only check + // assistant messages let fixture = Context::default() .add_message(TextMessage::new(Role::Assistant, "Response").model(ModelId::new("gpt-4"))) .add_message(TextMessage::new(Role::User, "Question").model(ModelId::new("claude-3"))); @@ -1680,8 +1686,8 @@ mod tests { #[test] fn test_has_model_changed_continuing_same_model() { - // Scenario: model1 -> model2 -> model2 (the second model2 should not drop - // reasoning) + // Scenario: model1 -> model2 -> model2 (the second model2 should not + // drop reasoning) let fixture = Context::default() .add_message(TextMessage::new(Role::Assistant, "First").model(ModelId::new("model1"))) .add_message(TextMessage::new(Role::User, "Question")) @@ -1706,8 +1712,9 @@ mod tests { /// extras. #[test] fn test_append_message_does_not_duplicate_reasoning_when_details_present() { - // Fixture: a structured reasoning detail with a valid signature, as would - // arrive after aggregating an Anthropic streaming response. + // Fixture: a structured reasoning detail with a valid signature, as + // would arrive after aggregating an Anthropic streaming + // response. let fixture_details = vec![ReasoningFull { text: Some("Let me think about this.".to_string()), signature: Some("EpwFvalidSignatureABC123".to_string()), @@ -1717,8 +1724,9 @@ mod tests { ..Default::default() }]; - // Both reasoning (raw string) and reasoning_details (structured) are provided, - // mirroring what orch.rs passes after collecting a streamed Anthropic response. + // Both reasoning (raw string) and reasoning_details (structured) are + // provided, mirroring what orch.rs passes after collecting a + // streamed Anthropic response. let fixture = Context::default().add_message(ContextMessage::user("Hello", None)); let actual = fixture.append_message( "Answer", diff --git a/crates/forge_domain/src/message.rs b/crates/forge_domain/src/message.rs index 38440ef061..ccf33a1f4a 100644 --- a/crates/forge_domain/src/message.rs +++ b/crates/forge_domain/src/message.rs @@ -408,7 +408,8 @@ mod tests { #[test] fn test_usage_merge_anthropic_cumulative() { // Fixture: Simulates Anthropic's message_start + message_delta pattern - // where output_tokens in message_delta is CUMULATIVE (total), not a delta. + // where output_tokens in message_delta is CUMULATIVE (total), not a + // delta. let fixture_message_start = Usage { prompt_tokens: TokenCount::Actual(1000), completion_tokens: TokenCount::Actual(1), // Initial output token diff --git a/crates/forge_domain/src/policies/engine.rs b/crates/forge_domain/src/policies/engine.rs index b89747a906..f9440a5f21 100644 --- a/crates/forge_domain/src/policies/engine.rs +++ b/crates/forge_domain/src/policies/engine.rs @@ -36,8 +36,8 @@ impl<'a> PolicyEngine<'a> { let mut last_allow: Option = None; - // Evaluate all policies in order: workflow policies first, then extended - // policies + // Evaluate all policies in order: workflow policies first, then + // extended policies if let Some(permission) = self.evaluate_policy_set(self.policies.policies.iter(), operation) { diff --git a/crates/forge_domain/src/policies/policy.rs b/crates/forge_domain/src/policies/policy.rs index 36ac5a344f..315276d2aa 100644 --- a/crates/forge_domain/src/policies/policy.rs +++ b/crates/forge_domain/src/policies/policy.rs @@ -37,7 +37,8 @@ impl Policy { } Policy::All { all: and } => { let permissions: Vec<_> = and.iter().map(|policy| policy.eval(operation)).collect(); - // For AND, we need all policies to pass, return the most restrictive permission + // For AND, we need all policies to pass, return the most + // restrictive permission permissions .into_iter() .find(|permission| permission.is_some()) @@ -53,7 +54,8 @@ impl Policy { } Policy::Not { not } => { let inner_permission = not.eval(operation); - // For NOT, invert the logic - if inner policy denies, we allow, and vice versa + // For NOT, invert the logic - if inner policy denies, we allow, + // and vice versa match inner_permission { Some(permission) => { let inverted_permission = match permission { diff --git a/crates/forge_domain/src/provider.rs b/crates/forge_domain/src/provider.rs index 72c914589f..0cdff67c96 100644 --- a/crates/forge_domain/src/provider.rs +++ b/crates/forge_domain/src/provider.rs @@ -974,7 +974,8 @@ mod tests { fn test_azure_provider() { let fixture = azure("test_key", "my-resource", "gpt-4", "2024-02-15-preview"); - // Check chat completion URL (url field now contains the chat completion URL) + // Check chat completion URL (url field now contains the chat completion + // URL) let actual_chat = fixture.url.clone(); let expected_chat = Url::parse("https://my-resource.openai.azure.com/openai/deployments/gpt-4/chat/completions?api-version=2024-02-15-preview").unwrap(); assert_eq!(actual_chat, expected_chat); diff --git a/crates/forge_domain/src/reasoning.rs b/crates/forge_domain/src/reasoning.rs index 8118477d03..89485019f0 100644 --- a/crates/forge_domain/src/reasoning.rs +++ b/crates/forge_domain/src/reasoning.rs @@ -110,7 +110,8 @@ mod tests { #[test] fn test_reasoning_detail_from_parts_groups_by_type() { - // Create a fixture with parts of different types across streaming deltas + // Create a fixture with parts of different types across streaming + // deltas let fixture = vec![ // First delta: reasoning.text vec![ReasoningPart { @@ -323,8 +324,8 @@ mod tests { let mut actual = Reasoning::from_parts(fixture); actual.sort_by(|a, b| a.type_of.cmp(&b.type_of)); // Sort by type for consistent ordering - // Now grouped by type: reasoning.text and reasoning.encrypted are separate - // entries + // Now grouped by type: reasoning.text and reasoning.encrypted are + // separate entries let mut expected = vec![ ReasoningFull { type_of: Some("reasoning.text".to_string()), diff --git a/crates/forge_domain/src/result_stream_ext.rs b/crates/forge_domain/src/result_stream_ext.rs index 46e08e7452..29c990ae1e 100644 --- a/crates/forge_domain/src/result_stream_ext.rs +++ b/crates/forge_domain/src/result_stream_ext.rs @@ -72,13 +72,14 @@ impl ResultStreamExt for crate::BoxStream 0 && *current_usage.completion_tokens > 0; @@ -89,7 +90,8 @@ impl ResultStreamExt for crate::BoxStream for crate::BoxStream None, }; } else { - // Merge partial usage using "max" strategy. This correctly handles - // providers like Anthropic where usage values are CUMULATIVE across - // events (message_start has input tokens, message_delta has the - // total output tokens). Using max instead of sum prevents - // double-counting when message_start includes output_tokens=1. + // Merge partial usage using "max" strategy. This correctly + // handles providers like Anthropic + // where usage values are CUMULATIVE across + // events (message_start has input tokens, message_delta has + // the total output tokens). Using max + // instead of sum prevents + // double-counting when message_start includes + // output_tokens=1. usage = usage.merge(current_usage); } } @@ -121,7 +126,8 @@ impl ResultStreamExt for crate::BoxStream for crate::BoxStream for crate::BoxStream for crate::BoxStream = messages .iter() .flat_map(|message| &message.tool_calls) @@ -214,8 +223,8 @@ impl ResultStreamExt for crate::BoxStream replace: usage = {1000, 1, 1001, 300} - // message_delta has prompt=0, completion=50 -> is_complete_usage=false -> - // merge: prompt = max(1000, 0) = 1000 + // message_delta has prompt=0, completion=50 -> is_complete_usage=false + // -> merge: prompt = max(1000, 0) = 1000 // completion = max(1, 50) = 50 (NOT 1+50=51) // total = max(1001, 50) = 1001 // cached = max(300, 0) = 300 @@ -602,7 +612,8 @@ mod tests { // Actual: Convert stream to full message let actual = result_stream.into_full(false).await.unwrap(); - // Expected: Usage should be merged from both MessageStart and MessageDelta + // Expected: Usage should be merged from both MessageStart and + // MessageDelta let expected = ChatCompletionMessageFull { content: "Hello world!".to_string(), tool_calls: vec![], @@ -787,7 +798,8 @@ mod tests { #[tokio::test] async fn test_into_full_with_reasoning() { - // Fixture: Create a stream with reasoning content across multiple messages + // Fixture: Create a stream with reasoning content across multiple + // messages let messages = vec![ Ok(ChatCompletionMessage::default() .content(Content::part("Hello ")) @@ -932,7 +944,8 @@ mod tests { // Actual: Convert stream to full message with XML interruption enabled let actual = result_stream.into_full(true).await.unwrap(); - // Expected: Should contain the XML tool call and final usage from last message + // Expected: Should contain the XML tool call and final usage from last + // message let expected_final_usage = Usage { prompt_tokens: TokenCount::Actual(5), completion_tokens: TokenCount::Actual(15), @@ -948,7 +961,8 @@ mod tests { #[tokio::test] async fn test_into_full_xml_tool_call_no_interruption_when_disabled() { - // Fixture: Create a stream with XML tool call content but interruption disabled + // Fixture: Create a stream with XML tool call content but interruption + // disabled let xml_content = r#" {"name": "test_tool", "arguments": {"arg": "value"}} "#; @@ -996,7 +1010,8 @@ mod tests { #[tokio::test] async fn test_into_full_usage_always_from_last_message_even_without_interruption() { - // Fixture: Create a stream where usage progresses through multiple messages + // Fixture: Create a stream where usage progresses through multiple + // messages let messages = vec![ Ok(ChatCompletionMessage::default().content(Content::part("Starting"))), Ok(ChatCompletionMessage::default().content(Content::part(" processing"))), @@ -1016,7 +1031,8 @@ mod tests { // Actual: Convert stream to full message let actual = result_stream.into_full(false).await.unwrap(); - // Expected: Usage should be from the last message (even if it has no content) + // Expected: Usage should be from the last message (even if it has no + // content) let expected = ChatCompletionMessageFull { content: "Starting processing complete".to_string(), tool_calls: vec![], @@ -1041,7 +1057,8 @@ mod tests { async fn test_into_full_with_finish_reason() { use crate::FinishReason; - // Fixture: Create a stream with multiple messages, some with finish reasons + // Fixture: Create a stream with multiple messages, some with finish + // reasons let messages = vec![ Ok(ChatCompletionMessage::default() .content(Content::part("Processing...")) @@ -1164,8 +1181,8 @@ mod tests { // Actual: Convert stream to full message with XML interruption enabled let actual = result_stream.into_full(true).await.unwrap(); - // Expected: Should have XML tool call, content only from before interruption, - // but final usage + // Expected: Should have XML tool call, content only from before + // interruption, but final usage assert_eq!(actual.content, xml_content); assert_eq!(actual.tool_calls.len(), 1); assert_eq!(actual.tool_calls[0].name.as_str(), "test_tool"); @@ -1177,8 +1194,8 @@ mod tests { async fn test_into_full_empty_completion_creates_unparsed_tool_calls() { use crate::Error; - // Fixture: Create a stream with empty content, no tool calls, and no finish - // reason + // Fixture: Create a stream with empty content, no tool calls, and no + // finish reason let messages = vec![ Ok(ChatCompletionMessage::default()), // Completely empty message Ok(ChatCompletionMessage::default().content(Content::part(""))), // Empty content diff --git a/crates/forge_domain/src/session_metrics.rs b/crates/forge_domain/src/session_metrics.rs index b3a25b3c80..5dcc7ccf72 100644 --- a/crates/forge_domain/src/session_metrics.rs +++ b/crates/forge_domain/src/session_metrics.rs @@ -150,7 +150,8 @@ mod tests { let actual = fixture; - // Check file1 has the last operation recorded (second add overwrites the first) + // Check file1 has the last operation recorded (second add overwrites + // the first) let file1_metrics = actual.file_operations.get("file1.rs").unwrap(); assert_eq!(file1_metrics.lines_added, 5); assert_eq!(file1_metrics.lines_removed, 1); diff --git a/crates/forge_domain/src/tool_order.rs b/crates/forge_domain/src/tool_order.rs index f07543a62c..7f62f1a205 100644 --- a/crates/forge_domain/src/tool_order.rs +++ b/crates/forge_domain/src/tool_order.rs @@ -347,8 +347,8 @@ mod tests { fixture_order.sort(&mut fixture); let actual: Vec = fixture.iter().map(|t| t.name.to_string()).collect(); - // read (pos 0), fs_read and fs_write (both match fs_* at pos 1, alphabetically - // sorted), shell (pos 2) + // read (pos 0), fs_read and fs_write (both match fs_* at pos 1, + // alphabetically sorted), shell (pos 2) let expected = vec!["read", "fs_read", "fs_write", "shell"]; assert_eq!(actual, expected); diff --git a/crates/forge_domain/src/tools/call/args.rs b/crates/forge_domain/src/tools/call/args.rs index 2a6bf77130..e1bc16fdda 100644 --- a/crates/forge_domain/src/tools/call/args.rs +++ b/crates/forge_domain/src/tools/call/args.rs @@ -20,7 +20,8 @@ impl Serialize for ToolCallArguments { { match self { ToolCallArguments::Unparsed(value) => { - // Use RawValue to serialize the JSON string without double serialization + // Use RawValue to serialize the JSON string without double + // serialization match RawValue::from_string(value.clone()) { Ok(raw) => raw.serialize(serializer), Err(_) => value.serialize(serializer), // Fallback if not valid JSON @@ -80,8 +81,9 @@ impl ToolCallArguments { if let Ok(repaired) = json_repair(&json_str) { ToolCallArguments::Parsed(repaired) } else { - // If it's not valid JSON, create a fallback object with the raw content - // This ensures we always send valid JSON to the API + // If it's not valid JSON, create a fallback object with the + // raw content This ensures we always + // send valid JSON to the API let mut map = Map::new(); map.insert("_raw_content".to_string(), Value::String(json_str)); ToolCallArguments::Parsed(Value::Object(map)) @@ -134,8 +136,8 @@ fn convert_string_to_value(value: &str) -> Value { } if let Ok(float_val) = value.parse::() { - // Create number from float, handling special case where float is actually an - // integer + // Create number from float, handling special case where float is + // actually an integer return if float_val.fract() == 0.0 { Value::Number(serde_json::Number::from(float_val as i64)) } else if let Some(num) = serde_json::Number::from_f64(float_val) { @@ -252,8 +254,8 @@ mod tests { fn test_serialize_unparsed_empty_string() { let fixture = ToolCallArguments::from_json(""); let actual = serde_json::to_string(&fixture).unwrap(); - // Empty string is not valid JSON, so it falls back to string serialization - // which produces a JSON string (quoted) + // Empty string is not valid JSON, so it falls back to string + // serialization which produces a JSON string (quoted) assert_eq!(actual, "\"\""); } diff --git a/crates/forge_domain/src/tools/call/parser.rs b/crates/forge_domain/src/tools/call/parser.rs index 899c0c3b24..b6c1e39939 100644 --- a/crates/forge_domain/src/tools/call/parser.rs +++ b/crates/forge_domain/src/tools/call/parser.rs @@ -107,7 +107,8 @@ pub fn parse(input: &str) -> Result, Error> { if tool_calls.is_empty() { return Err(Error::ToolCallParse(e.to_string())); } - // If we've already found some tool calls, we can stop here + // If we've already found some tool calls, we can stop + // here break; } } diff --git a/crates/forge_domain/src/tools/call/tool_call.rs b/crates/forge_domain/src/tools/call/tool_call.rs index 317906e6f6..94fc6b67f1 100644 --- a/crates/forge_domain/src/tools/call/tool_call.rs +++ b/crates/forge_domain/src/tools/call/tool_call.rs @@ -119,7 +119,8 @@ impl ToolCallFull { // - New/different call_id // - Empty name // - Partial arguments - // We need to associate these with the last tool call that had a valid name + // We need to associate these with the last tool call that had a valid + // name let mut last_valid_tool_name: Option = None; let mut last_valid_call_id: Option = None; @@ -128,8 +129,8 @@ impl ToolCallFull { let has_valid_name = part.name.as_ref().is_some_and(|n| !n.as_str().is_empty()); // GLM workaround: Detect GLM-style fragmented tool call - // Pattern: empty name + non-empty args + different call_id = continuation of - // previous tool + // Pattern: empty name + non-empty args + different call_id = + // continuation of previous tool let is_glm_fragment = !has_valid_name && !part.arguments_part.is_empty() && last_valid_tool_name.is_some() @@ -169,7 +170,8 @@ impl ToolCallFull { { current_tool_name = Some(name.clone()); last_valid_tool_name = Some(name.clone()); - // When we get a valid name, use the current call_id as the last valid one + // When we get a valid name, use the current call_id as the last + // valid one if let Some(ref cid) = current_call_id { last_valid_call_id = Some(cid.clone()); } @@ -213,8 +215,9 @@ impl ToolCallFull { args: content.to_string(), })?; - // User might switch the model from a tool unsupported to tool supported model - // leaving a lot of messages without tool calls + // User might switch the model from a tool unsupported to tool + // supported model leaving a lot of messages + // without tool calls tool_call.call_id = Some(ToolCallId::generate()); Ok(vec![tool_call]) @@ -456,8 +459,8 @@ mod tests { fn test_tool_in_both_failed_and_succeeded_lists() { let read = &ToolName::new("READ"); let mut counter = ToolErrorTracker::new(3); - // Tool appears in both failed and succeeded - success should NOT reset due to - // filter + // Tool appears in both failed and succeeded - success should NOT reset + // due to filter counter.adjust(&[read], &[read]); let actual = counter.maxed_out_tools(); @@ -577,8 +580,8 @@ mod tests { } #[test] fn test_try_from_parts_handles_empty_tool_names() { - // Fixture: Tool call parts where empty names in subsequent parts should not - // override valid names + // Fixture: Tool call parts where empty names in subsequent parts should + // not override valid names let input = [ ToolCallPart { call_id: Some(ToolCallId("0".to_string())), diff --git a/crates/forge_domain/src/tools/catalog.rs b/crates/forge_domain/src/tools/catalog.rs index ef911357c7..22638d258a 100644 --- a/crates/forge_domain/src/tools/catalog.rs +++ b/crates/forge_domain/src/tools/catalog.rs @@ -1154,7 +1154,8 @@ impl TryFrom for ToolCatalog { fn try_from(value: ToolCallFull) -> Result { // Normalize the tool name: trim whitespace and perform case-insensitive - // catalog match so the serde deserialization receives the canonical name. + // catalog match so the serde deserialization receives the canonical + // name. let normalized_name = normalize_tool_name(&value.name); let mut map = Map::new(); @@ -1265,7 +1266,8 @@ mod tests { fn test_coerce_string_integers_to_i32() { use crate::{ToolCallArguments, ToolCallFull}; - // Simulate the exact error case: read tool with string integers instead of i32 + // Simulate the exact error case: read tool with string integers instead + // of i32 let tool_call = ToolCallFull { name: ToolName::new("read"), call_id: None, diff --git a/crates/forge_domain/src/tools/definition/name.rs b/crates/forge_domain/src/tools/definition/name.rs index 58720ea1f2..f7d5261616 100644 --- a/crates/forge_domain/src/tools/definition/name.rs +++ b/crates/forge_domain/src/tools/definition/name.rs @@ -24,7 +24,8 @@ impl ToolName { let re_special = Regex::new(r"[^a-z0-9_]+").unwrap(); let cleaned = re_special.replace_all(&input, "_"); - // Remove leading/trailing underscores and collapse consecutive underscores + // Remove leading/trailing underscores and collapse consecutive + // underscores let re_trimmed = Regex::new(r"_+").unwrap(); let sanitized_str = re_trimmed diff --git a/crates/forge_domain/src/tools/definition/tool_definition.rs b/crates/forge_domain/src/tools/definition/tool_definition.rs index e33a3c005f..fab219c76b 100644 --- a/crates/forge_domain/src/tools/definition/tool_definition.rs +++ b/crates/forge_domain/src/tools/definition/tool_definition.rs @@ -148,7 +148,8 @@ mod tests { }; let actual = serde_json::to_value(&fixture).unwrap(); - // Titles must be absent at every level regardless of the schema structure + // Titles must be absent at every level regardless of the schema + // structure assert_eq!(actual.pointer("/input_schema/title"), None); assert_eq!( actual.pointer("/input_schema/$defs/NestedInput/title"), diff --git a/crates/forge_domain/src/transformer/normalize_tool_args.rs b/crates/forge_domain/src/transformer/normalize_tool_args.rs index fd3aaf25af..cd2573b3df 100644 --- a/crates/forge_domain/src/transformer/normalize_tool_args.rs +++ b/crates/forge_domain/src/transformer/normalize_tool_args.rs @@ -32,7 +32,8 @@ impl Transformer for NormalizeToolCallArguments { && let Some(ref mut tool_calls) = text_msg.tool_calls { for tool_call in tool_calls.iter_mut() { - // Normalize the arguments - converts Unparsed JSON strings to Parsed + // Normalize the arguments - converts Unparsed JSON strings + // to Parsed let args = std::mem::take(&mut tool_call.arguments); tool_call.arguments = args.normalize(); } @@ -52,7 +53,8 @@ mod tests { #[test] fn test_normalize_stringified_tool_call_arguments() { - // Create a context with stringified tool call arguments (like from old dump) + // Create a context with stringified tool call arguments (like from old + // dump) let context = Context::default() .add_message(ContextMessage::system("You are Forge.")) .add_message(ContextMessage::Text(TextMessage { diff --git a/crates/forge_domain/src/transformer/reasoning_normalizer.rs b/crates/forge_domain/src/transformer/reasoning_normalizer.rs index f29bc8ed1b..4110e12cc7 100644 --- a/crates/forge_domain/src/transformer/reasoning_normalizer.rs +++ b/crates/forge_domain/src/transformer/reasoning_normalizer.rs @@ -39,10 +39,10 @@ impl Transformer for ReasoningNormalizer { type Value = Context; fn transform(&mut self, mut context: Self::Value) -> Self::Value { - // Walk backwards to find the last assistant message (forward index) whose - // model differs from the current one. That is the cutoff: everything at - // or before it has reasoning stripped; the same-model tail after it is - // kept intact. + // Walk backwards to find the last assistant message (forward index) + // whose model differs from the current one. That is the + // cutoff: everything at or before it has reasoning stripped; + // the same-model tail after it is kept intact. let cutoff = context .messages .iter() @@ -278,7 +278,8 @@ mod tests { #[test] fn test_a_to_b_to_c_strips_reasoning() { - // A → B → C: every model switch must strip; here B→C triggers the strip. + // A → B → C: every model switch must strip; here B→C triggers the + // strip. let fixture = Context::default() .reasoning(ReasoningConfig::default().enabled(true)) .add_message(ContextMessage::user("q1", None)) @@ -293,8 +294,8 @@ mod tests { #[test] fn test_alternating_a_b_a_b_strips_reasoning() { - // A → B → A → B: after the full alternation the last assistant is model_a; - // switching to model_b must strip all reasoning. + // A → B → A → B: after the full alternation the last assistant is + // model_a; switching to model_b must strip all reasoning. let fixture = Context::default() .reasoning(ReasoningConfig::default().enabled(true)) .add_message(ContextMessage::user("q1", None)) @@ -312,8 +313,8 @@ mod tests { #[test] fn test_alternating_a_b_a_stay_a_strips_ab_keeps_last_a() { // A → B → A (stay on A): the cutoff is at b2 (the first mismatch going - // backwards), so a1 and b2 lose reasoning; only a3 (the same-model tail) - // is preserved. + // backwards), so a1 and b2 lose reasoning; only a3 (the same-model + // tail) is preserved. let fixture = Context::default() .reasoning(ReasoningConfig::default().enabled(true)) .add_message(ContextMessage::user("q1", None)) @@ -367,9 +368,10 @@ mod tests { fn test_mixed_sequence_preserves_only_same_model_tail() { // Sequence: a a a a b a c a b b b (current = b) // ↑↑↑ preserved tail - // ↑↑↑↑↑↑↑↑↑↑↑ stripped (everything before the tail break) - // The earlier `b` in the middle is also stripped because it is before - // the cutoff — only the contiguous tail from the end matters. + // ↑↑↑↑↑↑↑↑↑↑↑ stripped (everything before the tail + // break) The earlier `b` in the middle is also stripped because + // it is before the cutoff — only the contiguous tail from the + // end matters. let fixture = Context::default() .reasoning(ReasoningConfig::default().enabled(true)) .add_message(ContextMessage::user("q1", None)) diff --git a/crates/forge_domain/src/transformer/set_model.rs b/crates/forge_domain/src/transformer/set_model.rs index 8785ed742a..0bd84122fb 100644 --- a/crates/forge_domain/src/transformer/set_model.rs +++ b/crates/forge_domain/src/transformer/set_model.rs @@ -16,7 +16,8 @@ impl Transformer for SetModel { type Value = Context; fn transform(&mut self, mut value: Self::Value) -> Self::Value { - // Set the model for all text messages that don't already have a model set + // Set the model for all text messages that don't already have a model + // set for message in value.messages.iter_mut() { if let crate::ContextMessage::Text(text_msg) = &mut **message && text_msg.model.is_none() diff --git a/crates/forge_domain/src/transformer/sort_tools.rs b/crates/forge_domain/src/transformer/sort_tools.rs index 97fc4ce37e..3450f525fc 100644 --- a/crates/forge_domain/src/transformer/sort_tools.rs +++ b/crates/forge_domain/src/transformer/sort_tools.rs @@ -73,7 +73,8 @@ mod tests { let mut transformer = SortTools::new(custom_order); let actual = transformer.transform(fixture); - // zebra_tool and alpha_tool come first (in that order), rest alphabetically + // zebra_tool and alpha_tool come first (in that order), rest + // alphabetically let expected_order = vec!["zebra_tool", "alpha_tool", "beta_tool"]; let actual_order: Vec = actual .tools diff --git a/crates/forge_domain/src/transformer/transform_tool_calls.rs b/crates/forge_domain/src/transformer/transform_tool_calls.rs index da063a8886..a6f2142c52 100644 --- a/crates/forge_domain/src/transformer/transform_tool_calls.rs +++ b/crates/forge_domain/src/transformer/transform_tool_calls.rs @@ -21,9 +21,9 @@ impl Transformer for TransformToolCalls { type Value = Context; fn transform(&mut self, mut value: Self::Value) -> Self::Value { - // This transformer converts a tool-supported context to a non-tool-supported - // format We need to find assistant messages with tool calls and tool - // result messages + // This transformer converts a tool-supported context to a + // non-tool-supported format We need to find assistant messages + // with tool calls and tool result messages let mut new_messages = Vec::new(); diff --git a/crates/forge_domain/tests/test_stringified_tool_calls.rs b/crates/forge_domain/tests/test_stringified_tool_calls.rs index fd9d39ab2d..c7279b6be6 100644 --- a/crates/forge_domain/tests/test_stringified_tool_calls.rs +++ b/crates/forge_domain/tests/test_stringified_tool_calls.rs @@ -13,9 +13,9 @@ use forge_domain::{Context, ContextMessage, Role}; /// it should be a proper object. #[test] fn test_stringified_tool_call_arguments_roundtrip() { - // Simulate what kimi-k2p5-turbo sends: arguments as a string containing JSON - // Note: This is what the API sends us - a JSON string value containing JSON - // object + // Simulate what kimi-k2p5-turbo sends: arguments as a string containing + // JSON Note: This is what the API sends us - a JSON string value + // containing JSON object let conversation_json = r#"{ "messages": [ { @@ -46,7 +46,8 @@ fn test_stringified_tool_call_arguments_roundtrip() { ] }"#; - // Deserialize the conversation (this is what happens when we receive from API) + // Deserialize the conversation (this is what happens when we receive from + // API) let context: Context = serde_json::from_str(conversation_json).expect("Failed to parse conversation"); @@ -115,7 +116,8 @@ fn test_stringified_tool_call_arguments_roundtrip() { /// Test with patch tool (the actual error case from kimi-k2p5-turbo) #[test] fn test_kimi_k2p5_turbo_patch_tool_scenario() { - // This simulates the exact error case: patch tool with stringified arguments + // This simulates the exact error case: patch tool with stringified + // arguments let conversation_json = r#"{ "messages": [ { diff --git a/crates/forge_fs/src/binary_detection.rs b/crates/forge_fs/src/binary_detection.rs index d2c15213c7..100a5203c0 100644 --- a/crates/forge_fs/src/binary_detection.rs +++ b/crates/forge_fs/src/binary_detection.rs @@ -55,11 +55,12 @@ fn is_binary_internal(buffer: &[u8], bytes_read: usize) -> bool { let mut could_be_utf16be = true; // e.g. 0x00 0xAA let mut contains_zero_byte = false; - // This is a simplified guess to detect UTF-16 BE or LE by just checking if - // the first 512 bytes have the 0-byte at a specific location. For UTF-16 LE - // this would be the odd byte index and for UTF-16 BE the even one. - // Note: this can produce false positives (a binary file that uses a 2-byte - // encoding of the same format as UTF-16) and false negatives (a UTF-16 file + // This is a simplified guess to detect UTF-16 BE or LE by just checking + // if the first 512 bytes have the 0-byte at a specific + // location. For UTF-16 LE this would be the odd byte index and + // for UTF-16 BE the even one. Note: this can produce false + // positives (a binary file that uses a 2-byte encoding of the + // same format as UTF-16) and false negatives (a UTF-16 file // that is using 4 bytes to encode a character). const ZERO_BYTE_DETECTION_BUFFER_MAX_LEN: usize = 512; for (i, &byte) in buffer @@ -84,7 +85,8 @@ fn is_binary_internal(buffer: &[u8], bytes_read: usize) -> bool { could_be_utf16be = false; } - // Return if this is neither UTF16-LE nor UTF16-BE and thus treat as binary + // Return if this is neither UTF16-LE nor UTF16-BE and thus treat as + // binary if is_zero_byte && !could_be_utf16le && !could_be_utf16be { break; } @@ -152,7 +154,8 @@ mod tests { let fixture = create_test_file_fixture(&content).await?; let actual = is_binary(fixture.path()).await?; - // Should not detect as binary because zero byte is beyond 512-byte limit + // Should not detect as binary because zero byte is beyond 512-byte + // limit let expected = false; assert_eq!(actual, expected); Ok(()) diff --git a/crates/forge_infra/src/auth/mcp_token_storage.rs b/crates/forge_infra/src/auth/mcp_token_storage.rs index 50fde300d9..f0c5b09cc9 100644 --- a/crates/forge_infra/src/auth/mcp_token_storage.rs +++ b/crates/forge_infra/src/auth/mcp_token_storage.rs @@ -122,7 +122,8 @@ impl CredentialStore for McpTokenStorage { token_response .set_expires_in(Some(&std::time::Duration::from_secs(expires_at - now))); } else { - // Token has expired - set zero duration so rmcp triggers refresh + // Token has expired - set zero duration so rmcp triggers + // refresh token_response.set_expires_in(Some(&std::time::Duration::from_secs(0))); } } diff --git a/crates/forge_infra/src/auth/strategy.rs b/crates/forge_infra/src/auth/strategy.rs index 559a365f19..3e82317624 100644 --- a/crates/forge_infra/src/auth/strategy.rs +++ b/crates/forge_infra/src/auth/strategy.rs @@ -442,7 +442,8 @@ impl AuthStrategy for GoogleAdcStrategy { // 1. GOOGLE_APPLICATION_CREDENTIALS env var (service account) // 2. gcloud ADC credentials (user credentials) // 3. Metadata server (GCP environment) - // However, we still need to collect URL params like PROJECT_ID and LOCATION + // However, we still need to collect URL params like PROJECT_ID and + // LOCATION Ok(AuthContextRequest::ApiKey(ApiKeyRequest { required_params: self.required_params.clone(), existing_params: None, @@ -456,9 +457,10 @@ impl AuthStrategy for GoogleAdcStrategy { ) -> anyhow::Result { match context_response { AuthContextResponse::ApiKey(ctx) => { - // Validate that gcloud auth is properly configured before completing - // authentication This ensures the user has run 'gcloud auth - // application-default login' + // Validate that gcloud auth is properly configured before + // completing authentication This ensures the + // user has run 'gcloud auth application-default + // login' use google_cloud_auth::credentials::Builder; const VERTEX_AI_SCOPES: &[&str] = &["https://www.googleapis.com/auth/cloud-platform"]; @@ -483,7 +485,8 @@ impl AuthStrategy for GoogleAdcStrategy { // For Google ADC, we save a marker instead of the actual token // The token will be refreshed on every use - // But we still need to save the url_params (PROJECT_ID, LOCATION) + // But we still need to save the url_params (PROJECT_ID, + // LOCATION) Ok(AuthCredential::new_google_adc( self.provider_id.clone(), ApiKey::from("google_adc_marker".to_string()), /* Marker that will trigger @@ -706,8 +709,8 @@ impl AuthStrategy for CodexDeviceStrategy { chrono::Duration::hours(1), )?; - // Store account_id in url_params so it's persisted and available - // for chat request headers. + // Store account_id in url_params so it's persisted and + // available for chat request headers. enrich_codex_oauth_credential( &self.provider_id, &mut credential, @@ -748,7 +751,8 @@ async fn refresh_oauth_credential( ) } else { // No refresh token - use the existing long-lived OAuth access token - // This is typical for GitHub Copilot where the OAuth token is long-lived + // This is typical for GitHub Copilot where the OAuth token is + // long-lived tracing::debug!("No refresh token available, using existing OAuth access token"); ( tokens.access_token.to_string(), @@ -944,14 +948,16 @@ async fn codex_poll_for_tokens( let status = response.status(); if status.is_success() { - // Parse the custom response containing authorization_code + code_verifier + // Parse the custom response containing authorization_code + + // code_verifier let device_token: CodexDeviceTokenResponse = response.json().await.map_err(|e| { AuthError::PollFailed(format!("Failed to parse device token response: {e}")) })?; // Exchange the authorization code for OAuth tokens via standard - // endpoint. Use a clean HTTP client without custom headers since the - // standard OAuth token endpoint rejects unknown headers. + // endpoint. Use a clean HTTP client without custom headers since + // the standard OAuth token endpoint rejects unknown + // headers. let clean_client = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) .build() @@ -1180,7 +1186,8 @@ impl StrategyFactory for ForgeAuthStrategyFactory { ))) } forge_domain::AuthMethod::OAuthDevice(config) => { - // Check if this is OAuth-with-API-Key flow (GitHub Copilot pattern) + // Check if this is OAuth-with-API-Key flow (GitHub Copilot + // pattern) if config.token_refresh_url.is_some() { Ok(AnyAuthStrategy::OAuthWithApiKey( OAuthWithApiKeyStrategy::new(provider_id, config)?, diff --git a/crates/forge_infra/src/console.rs b/crates/forge_infra/src/console.rs index b7c84142f9..fb84f17c6a 100644 --- a/crates/forge_infra/src/console.rs +++ b/crates/forge_infra/src/console.rs @@ -99,9 +99,9 @@ mod tests { h1.join().unwrap(); h2.join().unwrap(); - // Verify output is one of the valid orderings where individual writes are - // atomic but sequences can interleave. AAAA must come before BBBB, XXXX - // must come before ZZZZ + // Verify output is one of the valid orderings where individual writes + // are atomic but sequences can interleave. AAAA must come + // before BBBB, XXXX must come before ZZZZ let actual = printer.stdout.lock().unwrap().get_ref().clone(); let valid_orderings = [ b"AAAABBBBXXXXZZZZ".to_vec(), // Thread 1 completes, then Thread 2 diff --git a/crates/forge_infra/src/env.rs b/crates/forge_infra/src/env.rs index 7a42705e51..6c64c9175d 100644 --- a/crates/forge_infra/src/env.rs +++ b/crates/forge_infra/src/env.rs @@ -133,7 +133,8 @@ impl EnvironmentInfra for ForgeEnvironmentInfra { } async fn update_environment(&self, ops: Vec) -> anyhow::Result<()> { - // Load the global config (with defaults applied) for the update round-trip + // Load the global config (with defaults applied) for the update + // round-trip let mut fc = ConfigReader::default() .read_defaults() .read_global() @@ -148,7 +149,8 @@ impl EnvironmentInfra for ForgeEnvironmentInfra { fc.write()?; debug!(config = ?fc, "written .forge.toml"); - // Reset cache so next get_config() re-reads the updated values from disk + // Reset cache so next get_config() re-reads the updated values from + // disk *self.cache.lock().expect("cache mutex poisoned") = None; Ok(()) @@ -193,8 +195,9 @@ mod tests { #[test] fn test_to_environment_falls_back_to_home_dir_when_env_var_absent() { let actual = to_environment(PathBuf::from("/any/cwd")); - // Without FORGE_CONFIG the base_path must be either ".forge" (new default) - // or "forge" (legacy fallback when ~/forge exists on this machine). + // Without FORGE_CONFIG the base_path must be either ".forge" (new + // default) or "forge" (legacy fallback when ~/forge exists on + // this machine). let name = actual.base_path.file_name().unwrap(); assert!( name == ".forge" || name == "forge", diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index af8718738d..31a32bad73 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -125,8 +125,9 @@ impl ForgeCommandExecutorService { )?; // If the command's stdout did not end with a newline, the terminal - // cursor is left mid-line. Write a newline so that subsequent output - // (e.g. the LLM response) starts on a fresh line. + // cursor is left mid-line. Write a newline so that subsequent + // output (e.g. the LLM response) starts on a fresh + // line. if result.1.last() != Some(&b'\n') && !result.1.is_empty() { let _ = self.output_printer.write(b"\n"); let _ = self.output_printer.flush(); @@ -191,8 +192,9 @@ async fn stream( let mut output = Vec::new(); if let Some(io) = io.as_mut() { let mut buff = [0; 1024]; - // Carry incomplete trailing UTF-8 codepoint bytes across reads — Windows - // console stdio rejects even one byte of a split codepoint. + // Carry incomplete trailing UTF-8 codepoint bytes across reads — + // Windows console stdio rejects even one byte of a split + // codepoint. let mut pending = Vec::::new(); loop { let n = io.read(&mut buff).await?; @@ -205,7 +207,8 @@ async fn stream( let mut working = std::mem::take(&mut pending); working.extend_from_slice(chunk); pending = write_lossy_utf8(&mut writer, &working)?; - // note: flush is necessary else we get the cursor could not be found error. + // note: flush is necessary else we get the cursor could not be + // found error. writer.flush()?; } // Flush dangling bytes from a stream that ended mid-codepoint. diff --git a/crates/forge_infra/src/kv_storage.rs b/crates/forge_infra/src/kv_storage.rs index f12d560312..031bdfaaf9 100644 --- a/crates/forge_infra/src/kv_storage.rs +++ b/crates/forge_infra/src/kv_storage.rs @@ -94,15 +94,17 @@ impl forge_app::KVStore for CacacheStorage { } Err(_) => { // Failed to deserialize (likely due to format change) - // Clear the invalid cache entry to maintain backward compatibility + // Clear the invalid cache entry to maintain backward + // compatibility let _ = cacache::remove(&self.cache_dir, &key_str).await; Ok(None) } } } Err(e) => { - // Check if error is NotFound by converting to string and checking message - // cacache errors don't have a kind() method + // Check if error is NotFound by converting to string and + // checking message cacache errors don't have a + // kind() method let error_str = e.to_string(); if error_str.contains("not found") || error_str.contains("NotFound") { Ok(None) diff --git a/crates/forge_infra/src/mcp_client.rs b/crates/forge_infra/src/mcp_client.rs index 1c0b5db47a..6c0c4d560f 100644 --- a/crates/forge_infra/src/mcp_client.rs +++ b/crates/forge_infra/src/mcp_client.rs @@ -132,7 +132,8 @@ impl ForgeMcpClient { self.create_oauth_connection(http, oauth_config, false) .await? } else { - // Auto-detect: try standard first, fall back to OAuth on auth errors + // Auto-detect: try standard first, fall back to OAuth on + // auth errors match self.create_standard_http_connection(http).await { Ok(client) => Arc::new(client), Err(e) => { @@ -147,8 +148,10 @@ impl ForgeMcpClient { "Standard connection failed with auth error for: {}, trying stored credentials", http.url ); - // Try OAuth with stored credentials (non-interactive) - // If stored credentials exist, use them; otherwise error + // Try OAuth with stored credentials + // (non-interactive) + // If stored credentials exist, use them; + // otherwise error let default_config = forge_domain::McpOAuthConfig::default(); self.create_oauth_connection(http, &default_config, false) .await? @@ -214,7 +217,8 @@ impl ForgeMcpClient { match auth_manager.initialize_from_store().await { Ok(true) => { // Stored credentials loaded. Try to get a valid access token - // (this auto-refreshes if expired and refresh_token is available) + // (this auto-refreshes if expired and refresh_token is + // available) match auth_manager.get_access_token().await { Ok(token) => { tracing::debug!("Using stored/refreshed OAuth token for: {}", http.url); @@ -259,8 +263,8 @@ impl ForgeMcpClient { let scopes: Vec<&str> = oauth_config.scopes.iter().map(|s| s.as_str()).collect(); - // start_authorization discovers metadata, registers client, generates PKCE + - // CSRF state + // start_authorization discovers metadata, registers client, generates + // PKCE + CSRF state oauth_state .start_authorization(&scopes, &redirect_uri, Some("Forge")) .await @@ -283,8 +287,9 @@ impl ForgeMcpClient { // Start local callback server, open browser, wait for redirect let (code, state) = self.run_oauth_callback_server(port, &auth_url).await?; - // Exchange authorization code for tokens (validates CSRF state internally) - // rmcp's OAuthState handles PKCE verifier inclusion in the token request + // Exchange authorization code for tokens (validates CSRF state + // internally) rmcp's OAuthState handles PKCE verifier inclusion + // in the token request oauth_state .handle_callback(&code, &state) .await diff --git a/crates/forge_json_repair/src/parser.rs b/crates/forge_json_repair/src/parser.rs index 35cc652b03..c207f305df 100644 --- a/crates/forge_json_repair/src/parser.rs +++ b/crates/forge_json_repair/src/parser.rs @@ -374,7 +374,8 @@ impl JsonRepairParser { // end of text, missing end quote let i_prev = self.prev_non_whitespace_index(self.i - 1); if !stop_at_delimiter && self.is_delimiter(self.chars.get(i_prev).copied()) { - // retry parsing the string, stopping at the first next delimiter + // retry parsing the string, stopping at the first next + // delimiter self.i = i_before; self.output.truncate(o_before); return self.parse_string(true, None); @@ -479,7 +480,8 @@ impl JsonRepairParser { self.i += 2; } ',' if skip_escape_chars => { - // Special case: escaped comma in escaped string should be treated as + // Special case: escaped comma in escaped string + // should be treated as // delimiter This creates a new array element str_content = self.insert_before_last_whitespace_str(&str_content, "\""); @@ -511,7 +513,8 @@ impl JsonRepairParser { // repair invalid unicode at end self.i = self.chars.len(); } else { - // SAFETY: j <= 6 and self.i + j < self.chars.len() (else branch + // SAFETY: j <= 6 and self.i + j < + // self.chars.len() (else branch // above handles >=) let invalid_chars: String = self .chars @@ -1014,7 +1017,8 @@ impl JsonRepairParser { index -= 1; } - // Convert the char-based index back to a byte offset for string slicing. + // Convert the char-based index back to a byte offset for string + // slicing. let byte_index = self .output .char_indices() @@ -1041,7 +1045,8 @@ impl JsonRepairParser { index -= 1; } - // Convert the char-based index back to a byte offset for string slicing. + // Convert the char-based index back to a byte offset for string + // slicing. let byte_index = text .char_indices() .nth(index) diff --git a/crates/forge_json_repair/src/schema_coercion.rs b/crates/forge_json_repair/src/schema_coercion.rs index 48fc0901f8..e1cc057cad 100644 --- a/crates/forge_json_repair/src/schema_coercion.rs +++ b/crates/forge_json_repair/src/schema_coercion.rs @@ -305,8 +305,9 @@ fn try_coerce_string( return Some(coerce_array_value(parsed, schema, root_schema)); } - // If direct parsing fails, try to extract array portion from the string - // This handles cases like: "[\"item\"]{\n}" or "garbage[\"item\"]more" + // If direct parsing fails, try to extract array portion from the + // string This handles cases like: "[\"item\"]{\n}" or + // "garbage[\"item\"]more" if let Some(extracted) = extract_array_from_string(s) { // Recursively coerce the extracted array items return Some(coerce_array_value(extracted, schema, root_schema)); @@ -410,7 +411,8 @@ fn parse_json_like_value(s: &str) -> Result { return Ok(parsed); } - // If that fails, try parsing as JSON5 (handles single quotes, comments, etc.) + // If that fails, try parsing as JSON5 (handles single quotes, comments, + // etc.) if let Ok(parsed) = serde_json5::from_str::(s) { return Ok(parsed); } @@ -764,7 +766,8 @@ mod tests { #[test] fn test_read_tool_line_numbers() { - // Simulate the exact case from the task: read tool with string line numbers + // Simulate the exact case from the task: read tool with string line + // numbers let fixture = json!({ "path": "/Users/amit/code-forge/crates/forge_main/src/ui.rs", "start_line": "2255", @@ -840,8 +843,8 @@ mod tests { let fixture = json!({"value": "2.14"}); let schema = schema_for!(IntOrNull); let actual = coerce_to_schema(fixture, &schema); - // The anyOf schema tries each subschema; since "2.14" can't be parsed as i64, - // it returns the original value + // The anyOf schema tries each subschema; since "2.14" can't be parsed + // as i64, it returns the original value let expected = json!({"value": "2.14"}); assert_eq!(actual, expected); } @@ -868,7 +871,8 @@ mod tests { #[test] fn test_array_with_tuple_schema_extra_items() { - // Test that Vec doesn't coerce items (no type constraints) + // Test that Vec doesn't coerce items (no type + // constraints) let fixture = json!({"items": ["1", "2", "3", "4"]}); let schema = schema_for!(ExtraItemsData); let actual = coerce_to_schema(fixture, &schema); @@ -994,8 +998,8 @@ mod tests { #[test] fn test_repairs_invalid_json_string_when_schema_expects_array() { - // Invalid JSON-like array strings are repaired into arrays when the schema - // expects one. + // Invalid JSON-like array strings are repaired into arrays when the + // schema expects one. let fixture = json!({"data": "[invalid json"}); let schema = schema_for!(DataArray); let actual = coerce_to_schema(fixture, &schema); @@ -1074,8 +1078,8 @@ mod tests { #[test] fn test_coerce_malformed_string_array_with_trailing_garbage() { - // This is the exact case from the issue: string that looks like an array but - // has trailing garbage + // This is the exact case from the issue: string that looks like an + // array but has trailing garbage let fixture = json!({ "tasks": "[\"Find where the main function is defined in the code-forge codebase. Search for main function definitions and entry points.\"]{\n}" }); @@ -1216,7 +1220,8 @@ mod tests { #[test] fn test_coerce_nested_array_with_string_numbers() { - // Test that nested coercion works - string numbers inside objects inside arrays + // Test that nested coercion works - string numbers inside objects + // inside arrays #[derive(JsonSchema)] #[allow(dead_code)] struct Item { @@ -1276,8 +1281,9 @@ mod tests { #[test] fn test_coerce_empty_string_to_null_for_nullable_field() { - // Simulates LLM sending "" for a nullable string field (e.g., file_type in - // fs_search). The schema uses "nullable: true" (OpenAPI 3.0 style). + // Simulates LLM sending "" for a nullable string field (e.g., file_type + // in fs_search). The schema uses "nullable: true" (OpenAPI 3.0 + // style). #[derive(JsonSchema)] #[allow(dead_code)] struct NullableStringData { @@ -1306,7 +1312,8 @@ mod tests { #[test] fn test_preserve_non_empty_string_for_nullable_field() { - // Non-empty strings should NOT be converted to null, even for nullable fields + // Non-empty strings should NOT be converted to null, even for nullable + // fields #[derive(JsonSchema)] #[allow(dead_code)] struct NullableStringData { diff --git a/crates/forge_json_repair/tests/error_cases.rs b/crates/forge_json_repair/tests/error_cases.rs index 5f8d6fefc5..d6b792687f 100644 --- a/crates/forge_json_repair/tests/error_cases.rs +++ b/crates/forge_json_repair/tests/error_cases.rs @@ -35,7 +35,8 @@ fn test_regex_single_slash() { #[test] fn test_regex_with_backslash_slash() { // Test regex with escaped slash at the end - // parse_regex treats the regex as a string literal, so backslash is preserved + // parse_regex treats the regex as a string literal, so backslash is + // preserved let fixture = r#"/a\/"#; let actual = json_repair::(fixture).unwrap(); let expected = serde_json::json!(r#"/a\/"#); @@ -55,11 +56,12 @@ fn test_string_with_colon_at_start() { #[test] fn test_multibyte_unicode_missing_end_quote() { // Triggers index out of bounds in insert_before_last_whitespace_str. - // The output buffer contains multi-byte UTF-8 characters (é = 2 bytes each), - // so self.output.len() (byte count) > chars.len() (char count). - // When the repair path calls insert_before_last_whitespace_str with trailing - // whitespace, it initialises `index` from the byte length and then indexes - // into a Vec at that byte-length position, panicking. + // The output buffer contains multi-byte UTF-8 characters (é = 2 bytes + // each), so self.output.len() (byte count) > chars.len() (char count). + // When the repair path calls insert_before_last_whitespace_str with + // trailing whitespace, it initialises `index` from the byte length and + // then indexes into a Vec at that byte-length position, + // panicking. let fixture = r#""café "#; let actual = json_repair::(fixture).unwrap(); let expected = serde_json::json!("café"); @@ -68,13 +70,14 @@ fn test_multibyte_unicode_missing_end_quote() { #[test] fn test_multibyte_unicode_missing_comma_in_object() { - // Triggers index out of bounds in insert_before_last_whitespace_str (line 459). - // parse_string first collects `"é,"` and hits the inner `"test"`. The - // prev_non_whitespace char is `,`, so it retries with stop_at_index=2 - // (the comma position). On retry it collects str_content = `"é` (3 bytes, - // 2 chars) and hits stop_at_index, calling insert_before_last_whitespace_str. - // That function sets index = text.len() = 3 (byte count) and then accesses - // chars[index - 1] = chars[2] on a Vec of length 2 — panic. + // Triggers index out of bounds in insert_before_last_whitespace_str (line + // 459). parse_string first collects `"é,"` and hits the inner `"test"`. + // The prev_non_whitespace char is `,`, so it retries with + // stop_at_index=2 (the comma position). On retry it collects + // str_content = `"é` (3 bytes, 2 chars) and hits stop_at_index, calling + // insert_before_last_whitespace_str. That function sets index = + // text.len() = 3 (byte count) and then accesses chars[index - 1] = + // chars[2] on a Vec of length 2 — panic. let fixture = "\"é,\"test\""; let actual = json_repair::(fixture).unwrap(); let expected = serde_json::json!(["é", "test"]); @@ -83,12 +86,12 @@ fn test_multibyte_unicode_missing_comma_in_object() { #[test] fn test_multibyte_unicode_missing_closing_brace() { - // Triggers index out of bounds in insert_before_last_whitespace_str (line 384). - // A string with a multi-byte character followed by trailing whitespace and - // no closing quote hits the "end of text, missing end quote" repair path. - // str_content = `"🎉 ` (6 bytes, 3 chars). insert_before_last_whitespace_str - // sets index = text.len() = 6 and accesses chars[5] on a Vec of - // length 3 — panic. + // Triggers index out of bounds in insert_before_last_whitespace_str (line + // 384). A string with a multi-byte character followed by trailing + // whitespace and no closing quote hits the "end of text, missing end + // quote" repair path. str_content = `"🎉 ` (6 bytes, 3 chars). + // insert_before_last_whitespace_str sets index = text.len() = 6 and + // accesses chars[5] on a Vec of length 3 — panic. let fixture = "\"🎉 "; let actual = json_repair::(fixture).unwrap(); let expected = serde_json::json!("🎉"); diff --git a/crates/forge_main/src/completer/command.rs b/crates/forge_main/src/completer/command.rs index 04daab5ec3..3ec8f0e821 100644 --- a/crates/forge_main/src/completer/command.rs +++ b/crates/forge_main/src/completer/command.rs @@ -28,10 +28,12 @@ impl CommandCompleter { impl CommandCompleter { pub fn complete(&mut self, line: &str, _: usize) -> Vec { - // Determine which sentinel the user typed (`:` or `/`), defaulting to `/`. + // Determine which sentinel the user typed (`:` or `/`), defaulting to + // `/`. let sentinel = if line.starts_with(':') { ':' } else { '/' }; - // Build the list of display names using the same sentinel the user typed. + // Build the list of display names using the same sentinel the user + // typed. let commands: Vec = self .0 .list() @@ -43,7 +45,8 @@ impl CommandCompleter { format!("{}{}", sentinel, cmd.name) }; - // Only include commands that match what the user has typed so far. + // Only include commands that match what the user has typed so + // far. if display_name.starts_with(line) { Some(CommandRow(ForgeCommand { name: display_name, @@ -60,8 +63,8 @@ impl CommandCompleter { return vec![]; } - // Extract the initial query text (everything after the leading sentinel or - // `!`). + // Extract the initial query text (everything after the leading sentinel + // or `!`). let initial_query = line .strip_prefix('/') .or_else(|| line.strip_prefix(':')) diff --git a/crates/forge_main/src/completer/input_completer.rs b/crates/forge_main/src/completer/input_completer.rs index d1df03d84b..ac48973a47 100644 --- a/crates/forge_main/src/completer/input_completer.rs +++ b/crates/forge_main/src/completer/input_completer.rs @@ -72,8 +72,8 @@ impl InputCompleter { pub fn complete(&mut self, line: &str, pos: usize) -> Vec { if line.starts_with('/') || line.starts_with(':') { - // if the line starts with '/' or ':' it's probably a command, so we delegate to - // the command completer. + // if the line starts with '/' or ':' it's probably a command, so we + // delegate to the command completer. let result = self.command.complete(line, pos); if !result.is_empty() { return result; diff --git a/crates/forge_main/src/conversation_selector.rs b/crates/forge_main/src/conversation_selector.rs index ea755621e6..a13a0d7f52 100644 --- a/crates/forge_main/src/conversation_selector.rs +++ b/crates/forge_main/src/conversation_selector.rs @@ -67,8 +67,8 @@ impl ConversationSelector { .add_key_value("Updated", time_ago); } - // Convert to porcelain, drop the UUID title column (col 0), truncate the - // Title column for display, uppercase headers + // Convert to porcelain, drop the UUID title column (col 0), truncate + // the Title column for display, uppercase headers let porcelain_output = Porcelain::from(&info) .drop_col(0) .truncate(0, 60) diff --git a/crates/forge_main/src/display_constants.rs b/crates/forge_main/src/display_constants.rs index 230bfb7578..15075d1ed8 100644 --- a/crates/forge_main/src/display_constants.rs +++ b/crates/forge_main/src/display_constants.rs @@ -134,7 +134,8 @@ mod tests { #[test] fn test_status_values_use_square_brackets() { - // Status values use square brackets to distinguish them from raw strings + // Status values use square brackets to distinguish them from raw + // strings assert!(status::YES.starts_with('[')); assert!(status::YES.ends_with(']')); assert!(status::NO.starts_with('[')); diff --git a/crates/forge_main/src/highlighter.rs b/crates/forge_main/src/highlighter.rs index 10919c3c75..baf4c099fa 100644 --- a/crates/forge_main/src/highlighter.rs +++ b/crates/forge_main/src/highlighter.rs @@ -32,9 +32,9 @@ impl ForgeHighlighter { return styled; } - // Command: highlight the command token (e.g. `:compact` or `/compact` for - // compat) in yellow bold, then the remainder (arguments) without - // special styling. + // Command: highlight the command token (e.g. `:compact` or `/compact` + // for compat) in yellow bold, then the remainder (arguments) + // without special styling. if line.starts_with('/') || line.starts_with(':') { let end = line.find(|c: char| c.is_whitespace()).unwrap_or(line.len()); styled.push(( @@ -55,8 +55,8 @@ impl ForgeHighlighter { return styled; } - // General message text — scan for `@[...]` file mentions and colour them cyan - // bold. + // General message text — scan for `@[...]` file mentions and colour + // them cyan bold. highlight_mentions(line, &mut styled); styled @@ -106,14 +106,16 @@ fn highlight_mentions(line: &str, styled: &mut StyledText) { match after_open.get(2..).and_then(|s| s.find(']')) { None => { // No closing `]` — emit `@[` and the rest as plain text - // to match ZSH behaviour (unterminated tag = no highlight). + // to match ZSH behaviour (unterminated tag = no + // highlight). styled.push((Style::new(), after_open.to_string())); break; } Some(rel_close) => { // Absolute position of `]` within `after_open`. let close = 2 + rel_close; - // Emit `@[...]` in cyan bold (inclusive of both brackets). + // Emit `@[...]` in cyan bold (inclusive of both + // brackets). let mention = after_open.get(..=close).unwrap_or(after_open); styled.push((Style::new().bold().fg(Color::Cyan), mention.to_string())); match after_open.get(close + 1..) { @@ -235,7 +237,8 @@ mod tests { #[test] fn test_file_mention_with_line_range() { - // @[path:start:end] — same as ZSH pattern, content inside can contain colons + // @[path:start:end] — same as ZSH pattern, content inside can contain + // colons let fixture = ForgeHighlighter; let actual = fixture.highlight("@[src/main.rs:10:20]", 0); assert_eq!(render(&actual), "@[src/main.rs:10:20]"); @@ -309,7 +312,8 @@ mod tests { #[test] fn test_bare_at_sign_not_highlighted() { - // A bare `@word` (no brackets) is plain text — matches ZSH pattern behaviour. + // A bare `@word` (no brackets) is plain text — matches ZSH pattern + // behaviour. let fixture = ForgeHighlighter; let actual = fixture.highlight("email@example.com", 0); assert_eq!(render(&actual), "email@example.com"); diff --git a/crates/forge_main/src/info.rs b/crates/forge_main/src/info.rs index b0815a8799..880df4eb31 100644 --- a/crates/forge_main/src/info.rs +++ b/crates/forge_main/src/info.rs @@ -551,8 +551,8 @@ pub(crate) fn format_path_for_display(env: &Environment, path: &Path) -> String { // Format based on OS return if env.os == "windows" { - // Use actual home path with proper quoting for Windows to work in both cmd and - // PowerShell + // Use actual home path with proper quoting for Windows to work in + // both cmd and PowerShell let home_path = home.display().to_string(); let full_path = format!( "{}{}{}", @@ -1085,8 +1085,9 @@ mod tests { let actual_str = String::from_utf8(stripped).unwrap(); // Verify that keys are padded within each section - // In SECTION ONE, all keys should be padded to length of "Very Long Key" (13) - // In SECTION TWO, all keys should be padded to length of "ABC" (3) + // In SECTION ONE, all keys should be padded to length of "Very Long + // Key" (13) In SECTION TWO, all keys should be padded to length + // of "ABC" (3) // Check that the display contains properly formatted sections assert!(actual_str.contains("SECTION ONE")); @@ -1160,8 +1161,8 @@ mod tests { let display = info.to_string(); - // All keys should be lowercase - checking just the key part without exact - // formatting + // All keys should be lowercase - checking just the key part without + // exact formatting assert!(display.contains("version")); assert!(display.contains("working directory")); assert!(display.contains("mixed case key")); @@ -1268,7 +1269,8 @@ mod tests { let actual = super::Info::from(&fixture); let expected_display = actual.to_string(); - // Verify it shows "No Changes Produced" when all files have zero changes + // Verify it shows "No Changes Produced" when all files have zero + // changes assert!(expected_display.contains("[No Changes Produced]")); assert!(!expected_display.contains("file1.rs")); assert!(!expected_display.contains("file2.rs")); diff --git a/crates/forge_main/src/main.rs b/crates/forge_main/src/main.rs index 7ad2b39be1..3d3cfedea1 100644 --- a/crates/forge_main/src/main.rs +++ b/crates/forge_main/src/main.rs @@ -143,7 +143,8 @@ mod tests { // We can't easily test the actual stdin reading in a unit test, // but we can verify the logic flow - // Test that when prompt is provided, it remains independent of piped input + // Test that when prompt is provided, it remains independent of piped + // input let cli_with_prompt = Cli::parse_from(["forge", "--prompt", "existing prompt"]); let original_prompt = cli_with_prompt.prompt.clone(); diff --git a/crates/forge_main/src/model.rs b/crates/forge_main/src/model.rs index aea647d14e..390793e371 100644 --- a/crates/forge_main/src/model.rs +++ b/crates/forge_main/src/model.rs @@ -173,7 +173,8 @@ impl ForgeCommandManager { pub fn register_all(&self, commands: Vec) { let mut guard = self.commands.lock().unwrap(); - // Remove existing workflow commands (those with ⚙ prefix in description) + // Remove existing workflow commands (those with ⚙ prefix in + // description) guard.retain(|cmd| !cmd.description.starts_with("⚙ ")); // Add new workflow commands @@ -324,7 +325,8 @@ impl ForgeCommandManager { match ClapCmd::try_parse_from(&argv) { Ok(mut cmd) => { - // Post-process variants that need Vec → concrete type fixup + // Post-process variants that need Vec → concrete type + // fixup match &mut cmd.sub { AppCommand::Commit { args, max_diff_size } => { *max_diff_size = args.iter().find_map(|p| p.parse::().ok()); @@ -344,10 +346,12 @@ impl ForgeCommandManager { } Err(clap_err) => { // Clap failed — check whether this is an agent command or a - // registered custom workflow command before surfacing the error. + // registered custom workflow command before surfacing the + // error. let command_name = bare; - // Give a domain-specific error for rename with no name argument. + // Give a domain-specific error for rename with no name + // argument. if (command_name == "rename" || command_name == "rn") && rest.is_empty() { return Err(anyhow::anyhow!( "Usage: :rename . Please provide a name for the conversation." @@ -387,7 +391,8 @@ impl ForgeCommandManager { )); } - // Surface a clean error from Clap (strips ANSI + internal parser name). + // Surface a clean error from Clap (strips ANSI + internal + // parser name). let rendered = clap_err.render().to_string(); let cleaned = rendered.replace("forge_cmd", "forge"); Err(anyhow::anyhow!("{}", cleaned.trim())) diff --git a/crates/forge_main/src/porcelain.rs b/crates/forge_main/src/porcelain.rs index 6454d1c206..ca85f9beb0 100644 --- a/crates/forge_main/src/porcelain.rs +++ b/crates/forge_main/src/porcelain.rs @@ -643,7 +643,8 @@ mod tests { ], ]); - // truncate(2, 5): "very_long_name" has 14 chars > 5, keep 5 then append "..." + // truncate(2, 5): "very_long_name" has 14 chars > 5, keep 5 then append + // "..." let actual = info.truncate(2, 5).into_rows(); let expected = vec![ @@ -664,7 +665,8 @@ mod tests { #[test] fn test_truncate_unicode_multibyte_chars() { - // Each emoji is 4 bytes but 1 char — byte-based truncation would misbehave here + // Each emoji is 4 bytes but 1 char — byte-based truncation would + // misbehave here let fixture = Porcelain(vec![vec![ Some("🦀🦀🦀🦀🦀🦀".into()), // 6 chars, 24 bytes Some("hi".into()), // 2 chars, under limit @@ -688,7 +690,8 @@ mod tests { #[test] fn test_truncate_unicode_exceeds_max_len() { - // 'é' is 2 bytes but 1 char — byte-based slicing would panic or cut wrong + // 'é' is 2 bytes but 1 char — byte-based slicing would panic or cut + // wrong let fixture = Porcelain(vec![vec![Some("héllo world".into())]]); // 11 chars let actual = fixture.truncate(0, 8).into_rows(); let expected = vec![vec![Some("héllo wo...".into())]]; // 8 chars kept + "..." diff --git a/crates/forge_main/src/sandbox.rs b/crates/forge_main/src/sandbox.rs index eee8daeadd..8d79a7cbd6 100644 --- a/crates/forge_main/src/sandbox.rs +++ b/crates/forge_main/src/sandbox.rs @@ -58,8 +58,8 @@ impl<'a> Sandbox<'a> { // Check if worktree already exists if worktree_path.exists() { - // Check if it's already a git worktree by checking if it has a .git file - // (worktree marker) + // Check if it's already a git worktree by checking if it has a .git + // file (worktree marker) let git_file = worktree_path.join(".git"); if git_file.exists() { let worktree_check = Command::new("git") diff --git a/crates/forge_main/src/stream_renderer.rs b/crates/forge_main/src/stream_renderer.rs index 881a8aecb2..0479665cb6 100644 --- a/crates/forge_main/src/stream_renderer.rs +++ b/crates/forge_main/src/stream_renderer.rs @@ -218,15 +218,17 @@ impl io::Write for StreamDirectWriter

{ self.printer.write(styled.as_bytes())?; self.printer.flush()?; - // Track if we ended on a newline - only safe to show spinner at line start + // Track if we ended on a newline - only safe to show spinner at line + // start if buf.last() == Some(&b'\n') { self.resume_spinner(); } - // Return `buf.len()`, not `styled.as_bytes().len()`. The `io::Write` contract - // requires returning how many bytes were consumed from the input buffer, not - // how many bytes were written to the output. Styling adds ANSI escape codes - // which makes the output larger than the input. + // Return `buf.len()`, not `styled.as_bytes().len()`. The `io::Write` + // contract requires returning how many bytes were consumed from + // the input buffer, not how many bytes were written to the + // output. Styling adds ANSI escape codes which makes the output + // larger than the input. Ok(buf.len()) } diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index a517907b24..f1b516cc5b 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -392,8 +392,8 @@ impl A + Send + Sync> UI } // Get initial input from prompt - // Prompt can fail if it doesn't have access to TTY. If it fails the first time, - // we will stop everything and bubble up the error. + // Prompt can fail if it doesn't have access to TTY. If it fails the + // first time, we will stop everything and bubble up the error. let mut command = self.prompt().await; loop { @@ -602,7 +602,8 @@ impl A + Send + Sync> UI // Read only the scope-specific config (not merged) let mut scope_config = self.api.read_mcp_config(Some(&scope)).await?; - // Merge the incoming servers with scope-specific config only + // Merge the incoming servers with scope-specific config + // only let mut added_servers = Vec::new(); for (server_name, server_config) in incoming_config.mcp_servers { scope_config @@ -718,7 +719,8 @@ impl A + Send + Sync> UI // Execute the custom command self.init_state(false).await?; - // If conversation_id is provided, set it in CLI before initializing + // If conversation_id is provided, set it in CLI before + // initializing if let Some(ref cid) = run_group.conversation_id { self.cli.conversation_id = Some(*cid); } @@ -1179,7 +1181,8 @@ impl A + Send + Sync> UI // Specific provider requested self.api.get_provider(id).await? } else { - // Fetch all providers for selection (no type filter, like shell :login) + // Fetch all providers for selection (no type filter, like shell + // :login) let providers = self.api.get_providers().await?; match self.select_provider_from_list(providers, "Provider", None, None)? { @@ -1223,8 +1226,8 @@ impl A + Send + Sync> UI return Ok(true); } - // Fetch and filter configured providers (like shell :logout filters to status - // [yes]) + // Fetch and filter configured providers (like shell :logout filters to + // status [yes]) let configured_providers: Vec = self .api .get_providers() @@ -1564,7 +1567,8 @@ impl A + Send + Sync> UI if porcelain { self.writeln(self.commands_porcelain().await?)?; } else { - // Non-porcelain: render in the same flat format as :help in the REPL. + // Non-porcelain: render in the same flat format as :help in the + // REPL. let command_manager = ForgeCommandManager::default(); command_manager.register_all(custom_commands); let info = Info::from(&command_manager); @@ -1679,7 +1683,8 @@ impl A + Send + Sync> UI // For porcelain mode, output raw TOML without highlighting self.writeln(config_toml)?; } else { - // For human-readable mode, add a title and syntax-highlight the TOML + // For human-readable mode, add a title and syntax-highlight the + // TOML self.writeln("\nCONFIGURATION\n".bold().dimmed())?; let highlighted = forge_display::SyntaxHighlighter::default().highlight(&config_toml, "toml"); @@ -2105,7 +2110,8 @@ impl A + Send + Sync> UI .map(|t| t.to_string()) .unwrap_or_else(|| markers::EMPTY.to_string()); - // Format time using humantime library (same as conversation_selector.rs) + // Format time using humantime library (same as + // conversation_selector.rs) let duration = chrono::Utc::now().signed_duration_since( conv.metadata.updated_at.unwrap_or(conv.metadata.created_at), ); @@ -2117,7 +2123,8 @@ impl A + Send + Sync> UI format!("{} ago", humantime::format_duration(duration)) }; - // Add conversation: Title=, Updated=<time_ago>, with ID as section title + // Add conversation: Title=<title>, Updated=<time_ago>, with ID as + // section title info = info .add_title(conv.id) .add_key_value("Title", title) @@ -2297,7 +2304,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI self.on_index(working_dir, false).await?; } AppCommand::AgentSwitch(agent_id) => { - // Validate that the agent exists by checking against loaded agents + // Validate that the agent exists by checking against loaded + // agents let agents = self.api.get_agent_infos().await?; let agent_exists = agents.iter().any(|agent| agent.id.as_str() == agent_id); @@ -3144,7 +3152,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI // Check if API key is already provided // For Google ADC, we use a marker to skip prompting - // For other providers, we use the existing key as a default value (autofill) + // For other providers, we use the existing key as a default value + // (autofill) let api_key_str = if let Some(default_key) = &request.api_key { let key_str = default_key.as_ref(); @@ -3165,7 +3174,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI api_key_str.to_string() } } else { - // For other providers, show the existing key as default (autofill) + // For other providers, show the existing key as default + // (autofill) let input = ForgeWidget::input(format!("Enter your {provider_id} API key")) .with_default(key_str); let api_key = input.prompt()?.context("API key input cancelled")?; @@ -3225,7 +3235,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI "→".blue(), display_uri.blue().underline() ))?; - // Try to copy code to clipboard automatically (not available on Android) + // Try to copy code to clipboard automatically (not available on + // Android) #[cfg(not(target_os = "android"))] let clipboard_copied = arboard::Clipboard::new() .and_then(|mut clipboard| clipboard.set_text(user_code)) @@ -3325,8 +3336,9 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI Some(server) } Ok(None) | Err(_) => { - // Not a localhost callback flow, or the listener could not be - // started — fall back to manual code paste. + // Not a localhost callback flow, or the listener could not + // be started — fall back to manual code + // paste. None } }; @@ -3639,9 +3651,9 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI } // Helper method to handle model selection and update the conversation. - // When `provider_filter` is `Some`, only models from that provider are shown. - // The model and provider returned by the selector are always set as one - // atomic operation. + // When `provider_filter` is `Some`, only models from that provider are + // shown. The model and provider returned by the selector are always set + // as one atomic operation. #[async_recursion::async_recursion] async fn on_model_selection( &mut self, @@ -3680,8 +3692,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI }; self.activate_provider(any_provider).await?; - // Check if provider was actually saved — if user cancelled model selection - // inside activate_provider, nothing was written + // Check if provider was actually saved — if user cancelled model + // selection inside activate_provider, nothing was written Ok(self.api.get_session_config().await.is_some()) } @@ -3699,7 +3711,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI any_provider: AnyProvider, model: Option<ModelId>, ) -> Result<()> { - // Trigger authentication for the selected provider only if not configured + // Trigger authentication for the selected provider only if not + // configured let provider = if !any_provider.is_configured() { match self .configure_provider(any_provider.id(), any_provider.auth_methods().to_vec()) @@ -3776,8 +3789,9 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI return Ok(()); } } else { - // The current model is compatible with the new provider — write both - // atomically so the session always stores a consistent pair. + // The current model is compatible with the new provider — write + // both atomically so the session always stores a + // consistent pair. let model = compatible_model.expect("compatible_model is Some when !needs_model_selection"); self.api @@ -3898,8 +3912,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI let active_agent = self.api.get_active_agent().await; // Validate provider is configured before loading agents - // If provider is set in config but not configured (no credentials), prompt user - // to login + // If provider is set in config but not configured (no credentials), + // prompt user to login if self.api.get_session_config().await.is_none() && !self.on_provider_selection().await? { return Ok(()); } @@ -3911,9 +3925,10 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI } if first { - // For chat, we are trying to get active agent or setting it to default. - // So for default values, `/info` doesn't show active provider, model, etc. - // So my default, on new, we should set the active agent. + // For chat, we are trying to get active agent or setting it to + // default. So for default values, `/info` doesn't show + // active provider, model, etc. So my default, on new, + // we should set the active agent. self.api .set_active_agent(active_agent.clone().unwrap_or_default()) .await?; @@ -3973,17 +3988,17 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI None => Event::empty(), }; - // Only use CLI piped_input as additional context when BOTH --prompt and piped - // input are provided. This handles the case: `echo "context" | forge -p - // "question"` where piped input provides context and --prompt provides - // the actual question. + // Only use CLI piped_input as additional context when BOTH --prompt and + // piped input are provided. This handles the case: `echo + // "context" | forge -p "question"` where piped input provides + // context and --prompt provides the actual question. // - // When only piped input is provided (no --prompt), it's already used as the - // main content (passed via the `content` parameter). We must NOT add it again - // as additional_context, otherwise the input appears twice in the - // conversation. We detect this by checking if cli.prompt exists - if it - // does, the content came from --prompt and piped input should be - // additional context. + // When only piped input is provided (no --prompt), it's already used as + // the main content (passed via the `content` parameter). We + // must NOT add it again as additional_context, otherwise the + // input appears twice in the conversation. We detect this by + // checking if cli.prompt exists - if it does, the content came + // from --prompt and piped input should be additional context. let piped_input = self.cli.piped_input.clone(); let has_explicit_prompt = self.cli.prompt.is_some(); if let Some(piped) = piped_input @@ -4064,7 +4079,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI // No related conversations, just render the main one conversation.to_html() } else { - // Render main conversation with related conversations in the same HTML + // Render main conversation with related conversations + // in the same HTML conversation.to_html_with_related(&related_conversations) }; @@ -4168,7 +4184,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI drop(_guard); } ChatResponse::ToolCallEnd(toolcall_result) => { - // Only track toolcall name in case of success else track the error. + // Only track toolcall name in case of success else track the + // error. let payload = if toolcall_result.is_error() { let mut r = ToolCallPayload::new(toolcall_result.name.to_string()); if let Some(cause) = toolcall_result.output.as_str() { @@ -4406,8 +4423,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI fn trace_user(&self) { let api = self.api.clone(); - // NOTE: Spawning required so that we don't block the user while querying user - // info + // NOTE: Spawning required so that we don't block the user while + // querying user info tokio::spawn(async move { if let Ok(Some(user_info)) = api.user_info().await { tracker::login(user_info.auth_provider_id.into_string()); @@ -4491,7 +4508,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI .await?; } ConfigSetField::Commit { provider, model } => { - // Validate provider exists and model belongs to that specific provider + // Validate provider exists and model belongs to that specific + // provider let validated_model = self.validate_model(model.as_str(), Some(&provider)).await?; let commit_config = forge_domain::ModelConfig::new(provider.clone(), validated_model.clone()); @@ -4504,7 +4522,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI )?; } ConfigSetField::Suggest { provider, model } => { - // Validate provider exists and model belongs to that specific provider + // Validate provider exists and model belongs to that specific + // provider let validated_model = self.validate_model(model.as_str(), Some(&provider)).await?; let suggest_config = forge_domain::ModelConfig::new(provider.clone(), validated_model.clone()); @@ -4626,7 +4645,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI None }; - // Check if nerd fonts should be used (NERD_FONT or USE_NERD_FONT set to "1") + // Check if nerd fonts should be used (NERD_FONT or USE_NERD_FONT set to + // "1") let use_nerd_font = std::env::var("NERD_FONT") .or_else(|_| std::env::var("USE_NERD_FONT")) .map(|val| val == "1") @@ -4882,7 +4902,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI self.spinner.stop(None)?; } - // Get active workspace ID if current workspace info is available + // Get active workspace ID if current workspace info is + // available let current_workspace = current_workspace_result.ok().flatten(); let active_workspace_id = current_workspace.as_ref().map(|ws| &ws.workspace_id); @@ -4896,7 +4917,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI // Output based on mode if porcelain { - // Skip header row in porcelain mode (consistent with conversation list) + // Skip header row in porcelain mode (consistent with + // conversation list) self.writeln(Porcelain::from(info).skip(1).drop_cols(&[0, 4, 5]))?; } else { self.writeln(info)?; @@ -4925,7 +4947,8 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI match workspace { Some(workspace) => { - // When viewing a specific workspace's info, it's implicitly the active one + // When viewing a specific workspace's info, it's implicitly the + // active one let mut info = Self::format_workspace_info(&workspace, true); // Add sync status summary if available diff --git a/crates/forge_main/src/vscode.rs b/crates/forge_main/src/vscode.rs index 01ee59468a..a040b2443a 100644 --- a/crates/forge_main/src/vscode.rs +++ b/crates/forge_main/src/vscode.rs @@ -139,9 +139,9 @@ mod tests { #[test] fn test_should_install_when_in_vscode() { with_env_var("TERM_PROGRAM", "vscode", || { - // We can't reliably test the actual installation check since it depends - // on the actual VS Code installation, but we can verify the logic - // when in VS Code terminal + // We can't reliably test the actual installation check since it + // depends on the actual VS Code installation, but we + // can verify the logic when in VS Code terminal assert!(is_vscode_terminal()); }); } diff --git a/crates/forge_main/src/zsh/paste.rs b/crates/forge_main/src/zsh/paste.rs index 72cb09ef50..7429384551 100644 --- a/crates/forge_main/src/zsh/paste.rs +++ b/crates/forge_main/src/zsh/paste.rs @@ -548,9 +548,9 @@ mod tests { assert_eq!(actual, expected); } - // Verifies that Cyrillic text with multi-byte UTF-8 characters doesn't panic - // when pasting. The original bug was caused by unsafe string slicing at byte - // boundaries inside multi-byte UTF-8 characters. + // Verifies that Cyrillic text with multi-byte UTF-8 characters doesn't + // panic when pasting. The original bug was caused by unsafe string + // slicing at byte boundaries inside multi-byte UTF-8 characters. #[test] fn test_wrap_pasted_text_cyrillic_no_crash() { let fixture = "Проверь ПОЛНОСТЬЮ этот проект на соответствие КАЖДОГО пункта функционала исходному тексту задачи"; @@ -558,8 +558,8 @@ mod tests { let actual = wrap_pasted_text(fixture); eprintln!("DEBUG: actual output = {:?}", actual); // The text should be preserved (it contains no absolute paths) - // The important thing is this doesn't panic with "byte index is not a char - // boundary" + // The important thing is this doesn't panic with "byte index is not a + // char boundary" assert!(!actual.is_empty()); assert!(actual.starts_with("Проверь")); } diff --git a/crates/forge_main/src/zsh/plugin.rs b/crates/forge_main/src/zsh/plugin.rs index 0350f9ad07..3f14593a23 100644 --- a/crates/forge_main/src/zsh/plugin.rs +++ b/crates/forge_main/src/zsh/plugin.rs @@ -19,8 +19,8 @@ static ZSH_PLUGIN_LIB: Dir<'static> = include_dir!("$CARGO_MANIFEST_DIR/../../sh pub fn generate_zsh_plugin() -> Result<String> { let mut output = String::new(); - // Iterate through all embedded files in shell-plugin/lib, stripping comments - // and empty lines. All files in this directory are .zsh files. + // Iterate through all embedded files in shell-plugin/lib, stripping + // comments and empty lines. All files in this directory are .zsh files. for file in forge_embed::files(&ZSH_PLUGIN_LIB) { let content = super::normalize_script(std::str::from_utf8(file.contents())?); for line in content.lines() { @@ -93,12 +93,12 @@ fn execute_zsh_script_with_streaming(script_content: &str, script_name: &str) -> // // On Windows, we write the script to a temp file and run `zsh -f <file>` // instead. A temp file is necessary because: - // 1. CI has core.autocrlf=true, so checked-out files contain CRLF; writing - // through normalize_script ensures the temp file has LF. - // 2. CreateProcess mangles quotes, so passing the script via -c corrupts any - // embedded quoting. - // 3. Piping via stdin is unreliable -- Windows caps pipe buffer size, which - // can truncate or block on larger scripts. + // 1. CI has core.autocrlf=true, so checked-out files contain CRLF; + // writing through normalize_script ensures the temp file has LF. + // 2. CreateProcess mangles quotes, so passing the script via -c corrupts + // any embedded quoting. + // 3. Piping via stdin is unreliable -- Windows caps pipe buffer size, + // which can truncate or block on larger scripts. // The -f flag also prevents ~/.zshrc from loading during execution. let (_temp_dir, mut child) = if cfg!(windows) { let (temp_dir, script_path) = create_temp_zsh_script(&script_content)?; @@ -320,8 +320,8 @@ pub fn setup_zsh_integration( } MarkerState::NotFound => { // No markers - add them at the end - // Add blank line before markers if file is not empty and doesn't end with blank - // line + // Add blank line before markers if file is not empty and doesn't + // end with blank line if lines.last().is_some_and(|l| !l.trim().is_empty()) { lines.push(String::new()); } @@ -336,7 +336,8 @@ pub fn setup_zsh_integration( // Generate timestamp for backup filename let timestamp = chrono::Local::now().format("%Y-%m-%d_%H-%M-%S"); - // Safe to unwrap: zshrc_path was constructed from a valid HOME/ZDOTDIR path + // Safe to unwrap: zshrc_path was constructed from a valid HOME/ZDOTDIR + // path let parent = zshrc_path .parent() .context("zshrc path has no parent directory")?; @@ -379,7 +380,8 @@ mod tests { /// (e.g., plugin not loaded), or zsh may not be available in CI #[test] fn test_run_zsh_doctor_streaming() { - // SAFETY: No mutex needed for single test - setting env var for test isolation + // SAFETY: No mutex needed for single test - setting env var for test + // isolation unsafe { std::env::set_var("FORGE_SKIP_INTERACTIVE", "1"); } @@ -400,8 +402,8 @@ mod tests { // Success case } Err(e) => { - // Check if it's a non-zero exit code error or zsh not available (both expected - // in tests) + // Check if it's a non-zero exit code error or zsh not available + // (both expected in tests) let error_msg = e.to_string(); assert!( error_msg.contains("exit code") || error_msg.contains("Failed to execute"), @@ -465,7 +467,8 @@ mod tests { let original_home = std::env::var("HOME").ok(); let original_zdotdir = std::env::var("ZDOTDIR").ok(); - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { std::env::set_var("HOME", temp_dir.path()); std::env::remove_var("ZDOTDIR"); @@ -475,7 +478,8 @@ mod tests { let actual = setup_zsh_integration(false, None); // Restore environment first - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { if let Some(home) = original_home { std::env::set_var("HOME", home); @@ -522,7 +526,8 @@ mod tests { let original_home = std::env::var("HOME").ok(); let original_zdotdir = std::env::var("ZDOTDIR").ok(); - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { std::env::set_var("HOME", temp_dir.path()); std::env::set_var("ZDOTDIR", temp_dir.path()); @@ -555,7 +560,8 @@ mod tests { assert!(content.contains("# <<< forge initialize <<<")); // Restore environment - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { if let Some(home) = original_home { std::env::set_var("HOME", home); @@ -581,7 +587,8 @@ mod tests { let original_home = std::env::var("HOME").ok(); let original_zdotdir = std::env::var("ZDOTDIR").ok(); - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { std::env::set_var("HOME", temp_dir.path()); std::env::remove_var("ZDOTDIR"); @@ -616,7 +623,8 @@ mod tests { assert!(content.contains("# <<< forge initialize <<<")); // Restore environment - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { if let Some(home) = original_home { std::env::set_var("HOME", home); @@ -646,7 +654,8 @@ mod tests { let original_home = std::env::var("HOME").ok(); let original_zdotdir = std::env::var("ZDOTDIR").ok(); - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { std::env::set_var("HOME", temp_dir.path()); std::env::set_var("ZDOTDIR", temp_dir.path()); @@ -677,7 +686,8 @@ mod tests { assert!(content.contains("# <<< forge initialize <<<")); // Restore environment - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { if let Some(home) = original_home { std::env::set_var("HOME", home); @@ -703,7 +713,8 @@ mod tests { let original_home = std::env::var("HOME").ok(); let original_zdotdir = std::env::var("ZDOTDIR").ok(); - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { std::env::set_var("HOME", temp_dir.path()); std::env::remove_var("ZDOTDIR"); @@ -785,7 +796,8 @@ mod tests { ); // Restore environment - // SAFETY: We hold ENV_LOCK to prevent concurrent environment modifications + // SAFETY: We hold ENV_LOCK to prevent concurrent environment + // modifications unsafe { if let Some(home) = original_home { std::env::set_var("HOME", home); diff --git a/crates/forge_markdown_stream/src/renderer.rs b/crates/forge_markdown_stream/src/renderer.rs index 7b4962c979..12d2f94c18 100644 --- a/crates/forge_markdown_stream/src/renderer.rs +++ b/crates/forge_markdown_stream/src/renderer.rs @@ -256,7 +256,8 @@ impl<W: Write> Renderer<W> { ParseEvent::BlockquoteLine(text) => { let margin = self.left_margin(); let content_width = self.width.saturating_sub(visible_length(&margin)); - // Parse inline formatting (bold, italic, etc.) in blockquote content + // Parse inline formatting (bold, italic, etc.) in blockquote + // content let rendered_content = render_inline_content(text, &self.theme); let wrapped = wrap_text_preserving_spaces( &rendered_content, diff --git a/crates/forge_markdown_stream/src/table.rs b/crates/forge_markdown_stream/src/table.rs index 0806974fd2..9b98cec3e0 100644 --- a/crates/forge_markdown_stream/src/table.rs +++ b/crates/forge_markdown_stream/src/table.rs @@ -256,7 +256,8 @@ fn wrap(text: &str, width: usize) -> Vec<String> { line = active_style.clone().unwrap_or_default(); line_width = 0; } - // Push the long word, breaking at width while preserving ANSI codes + // Push the long word, breaking at width while preserving ANSI + // codes while visible_length(&word) > width { let (chunk, rem) = split_word_at_width(&word, width); if !chunk.is_empty() { diff --git a/crates/forge_repo/src/agent.rs b/crates/forge_repo/src/agent.rs index 313d759a85..d857105a2a 100644 --- a/crates/forge_repo/src/agent.rs +++ b/crates/forge_repo/src/agent.rs @@ -124,8 +124,8 @@ fn resolve_agent_conflicts(agents: Vec<AgentDefinition>) -> Vec<AgentDefinition> agent_map.insert(agent.id.to_string(), agent); } - // Convert back to vector (order is not guaranteed but doesn't matter for the - // service) + // Convert back to vector (order is not guaranteed but doesn't matter for + // the service) agent_map.into_values().collect() } diff --git a/crates/forge_repo/src/conversation/conversation_repo.rs b/crates/forge_repo/src/conversation/conversation_repo.rs index eeef25af71..cd728b3ba4 100644 --- a/crates/forge_repo/src/conversation/conversation_repo.rs +++ b/crates/forge_repo/src/conversation/conversation_repo.rs @@ -134,7 +134,8 @@ impl ConversationRepository for ConversationRepositoryImpl { self.run_with_connection(move |connection, wid| { let workspace_id = wid.id() as i64; - // Security: Ensure users can only delete conversations within their workspace + // Security: Ensure users can only delete conversations within their + // workspace diesel::delete(conversations::table) .filter(conversations::workspace_id.eq(&workspace_id)) .filter(conversations::conversation_id.eq(conversation_id.into_string())) @@ -886,9 +887,10 @@ mod tests { repo.upsert_conversation(conversation.clone()).await?; - // Try to delete with different workspace ID (should fail due to security) - // Note: This test would require modifying workspace ID in repo - // For now, we test that deletion works with current workspace + // Try to delete with different workspace ID (should fail due to + // security) Note: This test would require modifying workspace + // ID in repo For now, we test that deletion works with current + // workspace repo.delete_conversation(&conversation.id).await?; // Verify it's actually deleted @@ -904,7 +906,8 @@ mod tests { let conversation = Conversation::new(conversation_id).title(Some("Test Conversation".to_string())); - // Test complete workflow: create -> delete -> verify -> create new -> verify + // Test complete workflow: create -> delete -> verify -> create new -> + // verify repo.upsert_conversation(conversation.clone()).await?; // Delete conversation @@ -967,8 +970,8 @@ mod tests { fn test_legacy_tool_value_pair_deserialization() { use crate::conversation::conversation_record::ToolOutputRecord; - // This JSON represents the old Pair variant format that was stored in the - // database + // This JSON represents the old Pair variant format that was stored in + // the database let legacy_json = r#"{ "is_error": false, "values": [ diff --git a/crates/forge_repo/src/provider/anthropic.rs b/crates/forge_repo/src/provider/anthropic.rs index 9bcb906cb9..72fb10d46c 100644 --- a/crates/forge_repo/src/provider/anthropic.rs +++ b/crates/forge_repo/src/provider/anthropic.rs @@ -40,8 +40,8 @@ impl<H: HttpInfra> Anthropic<H> { self.anthropic_version.clone(), )]; - // Extract API key/token from provider credentials (handles Google ADC, OAuth, - // and API key) + // Extract API key/token from provider credentials (handles Google ADC, + // OAuth, and API key) let api_key = self .provider .credential @@ -126,7 +126,8 @@ impl<T: HttpInfra> Anthropic<T> { let mut request = Request::try_from(context)?.max_tokens(max_tokens as u64); - // For Vertex AI Anthropic, model is in the URL path, not the request body + // For Vertex AI Anthropic, model is in the URL path, not the request + // body if self.provider.id == ProviderId::VERTEX_AI_ANTHROPIC { request = request.anthropic_version(self.anthropic_version.clone()); } else { @@ -140,8 +141,8 @@ impl<T: HttpInfra> Anthropic<T> { .pipe(DropInvalidToolUse) .pipe(SanitizeToolIds); - // Vertex AI does not support output_format, so we skip schema enforcement - // and remove any output_format field + // Vertex AI does not support output_format, so we skip schema + // enforcement and remove any output_format field let request = if self.provider.id == ProviderId::VERTEX_AI_ANTHROPIC { pipeline .pipe(RemoveOutputFormat) @@ -155,8 +156,9 @@ impl<T: HttpInfra> Anthropic<T> { }; let url = if self.provider.id == ProviderId::VERTEX_AI_ANTHROPIC { - // For Vertex AI, we need to append the model ID and streamRawPredict to the URL - // The chat_url from provider.json ends with .../models + // For Vertex AI, we need to append the model ID and + // streamRawPredict to the URL The chat_url from + // provider.json ends with .../models let base = self.provider.url.as_str().trim_end_matches('/'); format!("{}/{}:streamRawPredict", base, model.as_str()) } else { @@ -448,7 +450,8 @@ mod tests { _headers: Option<HeaderMap>, _body: Bytes, ) -> anyhow::Result<EventSource> { - // For now, return an error since eventsource is not used in the failing tests + // For now, return an error since eventsource is not used in the + // failing tests Err(anyhow::anyhow!("EventSource not implemented in mock")) } } @@ -809,7 +812,8 @@ mod tests { #[test] fn test_get_headers_drops_interleaved_thinking_for_4_6_plus_models() { // Adaptive thinking auto-enables interleaved thinking on Opus 4.8, - // Opus 4.7, Opus 4.6, and Sonnet 4.6; the beta header is redundant there. + // Opus 4.7, Opus 4.6, and Sonnet 4.6; the beta header is redundant + // there. let chat_url = Url::parse("https://api.anthropic.com/v1/messages").unwrap(); let model_url = Url::parse("https://api.anthropic.com/v1/models").unwrap(); diff --git a/crates/forge_repo/src/provider/bedrock.rs b/crates/forge_repo/src/provider/bedrock.rs index 7b3689c8e0..a39364ee0e 100644 --- a/crates/forge_repo/src/provider/bedrock.rs +++ b/crates/forge_repo/src/provider/bedrock.rs @@ -144,7 +144,8 @@ impl BedrockProvider { _ => "", }; - // Only prefix Anthropic models that don't already have a regional prefix + // Only prefix Anthropic models that don't already have a regional + // prefix if model_id.contains("anthropic.") && !model_id.starts_with("us.") && !model_id.starts_with("eu.") @@ -236,15 +237,17 @@ impl BedrockProvider { .map_err(|sdk_error| { use aws_sdk_bedrockruntime::error::SdkError; - // Check if this is a retryable error by matching on SDK error types + // Check if this is a retryable error by matching on SDK error + // types let is_retryable = match &sdk_error { SdkError::ServiceError(err) => Self::is_retryable_converse_error(err.err()), _ => Self::is_retryable_sdk_error(&sdk_error), }; // Extract the source error for better error messages - // SAFETY: into_source() always returns Ok for all SdkError variants - // (see aws-smithy-runtime-api/src/client/result.rs:448-459) + // SAFETY: into_source() always returns Ok for all SdkError + // variants (see aws-smithy-runtime-api/src/ + // client/result.rs:448-459) let source = sdk_error.into_source().unwrap(); if is_retryable { @@ -265,7 +268,8 @@ impl BedrockProvider { Err(stream_error) => { use aws_sdk_bedrockruntime::error::SdkError; - // Check if this is a retryable stream error by matching on SDK error types + // Check if this is a retryable stream error by matching on + // SDK error types let is_retryable = match &stream_error { SdkError::ServiceError(err) => { Self::is_retryable_stream_output_error(err.err()) @@ -413,8 +417,9 @@ impl IntoDomain for aws_sdk_bedrockruntime::types::ConverseStreamOutput { ConverseStreamOutput::Metadata(metadata) => { // Metadata contains usage information let usage = metadata.usage.map(|u| { - // AWS Bedrock supports cache tokens but not reasoning tokens - // Sum both cache read and cache write tokens into cached_tokens field + // AWS Bedrock supports cache tokens but not reasoning + // tokens Sum both cache read and cache + // write tokens into cached_tokens field let cached_tokens = u .cache_read_input_tokens .unwrap_or(0) @@ -453,12 +458,14 @@ impl FromDomain<forge_domain::Context> use aws_sdk_bedrockruntime::operation::converse_stream::ConverseStreamInput; use aws_sdk_bedrockruntime::types::{InferenceConfiguration, Message, SystemContentBlock}; - // Capture reasoning-related flags before `context.messages` / other fields - // are consumed below. `ModelSpecificReasoning` runs earlier in the pipeline - // and has already normalized `reasoning` per model family, so here we just - // branch on the shape it produced: + // Capture reasoning-related flags before `context.messages` / other + // fields are consumed below. `ModelSpecificReasoning` runs + // earlier in the pipeline and has already normalized + // `reasoning` per model family, so here we just branch on the + // shape it produced: // - `max_tokens.is_some()` -> legacy `thinking.enabled` budget shape - // - otherwise -> `thinking.adaptive` (Opus 4.7 / 4.6 / Sonnet 4.6) + // - otherwise -> `thinking.adaptive` (Opus 4.7 / 4.6 / + // Sonnet 4.6) let reasoning_on = context.is_reasoning_supported(); let emits_legacy_thinking = reasoning_on && context @@ -482,8 +489,8 @@ impl FromDomain<forge_domain::Context> .collect(); // Convert user and assistant messages - // Group consecutive tool results into single User messages as required by - // Bedrock API + // Group consecutive tool results into single User messages as required + // by Bedrock API let messages: Vec<Message> = { let mut result = Vec::new(); let mut pending_tool_results: Vec<forge_domain::ContextMessage> = Vec::new(); @@ -499,7 +506,8 @@ impl FromDomain<forge_domain::Context> pending_tool_results.push(message.message); } _ => { - // Flush pending tool results before processing non-tool message + // Flush pending tool results before processing non-tool + // message if !pending_tool_results.is_empty() { let tool_results: Vec<_> = std::mem::take(&mut pending_tool_results); result.push(Message::from_domain(tool_results)?); @@ -551,11 +559,13 @@ impl FromDomain<forge_domain::Context> // Convert inference configuration // When `thinking.enabled` (legacy budget shape) is being emitted below, - // Anthropic-on-Bedrock requires `top_p >= 0.95` or unset. `thinking.adaptive` - // (Opus 4.7 / Opus 4.6 / Sonnet 4.6) has no such constraint, and - // `ModelSpecificReasoning` already strips `top_p` entirely for Opus 4.7. + // Anthropic-on-Bedrock requires `top_p >= 0.95` or unset. + // `thinking.adaptive` (Opus 4.7 / Opus 4.6 / Sonnet 4.6) has no + // such constraint, and `ModelSpecificReasoning` already strips + // `top_p` entirely for Opus 4.7. let adjusted_top_p = if emits_legacy_thinking { - // If legacy thinking is emitted and top_p is set, ensure it's at least 0.95 + // If legacy thinking is emitted and top_p is set, ensure it's at + // least 0.95 context.top_p.map(|p| { let value = p.value(); if value < 0.95 { @@ -590,11 +600,11 @@ impl FromDomain<forge_domain::Context> // `reasoning.max_tokens`, which `ModelSpecificReasoning` has already // normalized per family: // - // - `max_tokens: Some(N)` → `{type: "enabled", budget_tokens: N}` (Opus 4.5 - // and older; budget is backfilled to 10k when absent.) - // - `max_tokens: None` → `{type: "adaptive", display: ...}` (Opus 4.7 - // rejects the legacy shape with 400; Opus 4.6 / Sonnet 4.6 accept adaptive - // natively.) + // - `max_tokens: Some(N)` → `{type: "enabled", budget_tokens: N}` + // (Opus 4.5 and older; budget is backfilled to 10k when absent.) + // - `max_tokens: None` → `{type: "adaptive", display: ...}` (Opus + // 4.7 rejects the legacy shape with 400; Opus 4.6 / Sonnet 4.6 + // accept adaptive natively.) // // When present, `reasoning.effort` is emitted as `output_config.effort` // for families that support it (`ModelSpecificReasoning` drops effort @@ -761,8 +771,9 @@ impl FromDomain<forge_domain::ContextMessage> for aws_sdk_bedrockruntime::types: forge_domain::ContextMessage::Text(text_msg) => { let mut content_blocks = Vec::new(); - // Add thought signature FIRST if present (for Assistant messages) - // AWS requires that when thinking is enabled, assistant messages MUST start + // Add thought signature FIRST if present (for Assistant + // messages) AWS requires that when thinking is + // enabled, assistant messages MUST start // with reasoning blocks if text_msg.role == forge_domain::Role::Assistant && let Some(reasoning_details) = &text_msg.reasoning_details @@ -1031,8 +1042,9 @@ impl FromDomain<forge_domain::ToolChoice> for aws_sdk_bedrockruntime::types::Too .map_err(|e| anyhow::anyhow!("Failed to build tool choice: {}", e))?, ), forge_domain::ToolChoice::None => { - // For None, we'll return a default Auto choice, but the caller should handle - // this by not setting tool_choice at all + // For None, we'll return a default Auto choice, but the caller + // should handle this by not setting tool_choice + // at all ToolChoice::Auto(AutoToolChoice::builder().build()) } }; diff --git a/crates/forge_repo/src/provider/chat.rs b/crates/forge_repo/src/provider/chat.rs index e8c1919970..9608f81868 100644 --- a/crates/forge_repo/src/provider/chat.rs +++ b/crates/forge_repo/src/provider/chat.rs @@ -89,7 +89,8 @@ impl<F: EnvironmentInfra<Config = forge_config::ForgeConfig> + HttpInfra + Sync> tracing::debug!(provider_id = %provider.id, "returning cached models; refreshing in background"); // Spawn a background task to refresh the disk cache. The abort - // handle is stored so the task is cancelled if the service is dropped. + // handle is stored so the task is cancelled if the service is + // dropped. let cache = self.model_cache.clone(); let router = self.router.clone(); let key = cache_key; diff --git a/crates/forge_repo/src/provider/google.rs b/crates/forge_repo/src/provider/google.rs index c70b2b28c8..88e817116f 100644 --- a/crates/forge_repo/src/provider/google.rs +++ b/crates/forge_repo/src/provider/google.rs @@ -399,10 +399,11 @@ mod tests { }]) .tool_choice(ToolChoice::Call(ToolName::new("math"))); - // We can't easily test Request::from(context) directly here because Request is - // private or we need to access it via Google::chat But we can check the - // serialized request if we mock the http call. However, Request is pub - // in dto::google::Request, so we can use it if we import it. The import + // We can't easily test Request::from(context) directly here because + // Request is private or we need to access it via Google::chat + // But we can check the serialized request if we mock the http + // call. However, Request is pub in dto::google::Request, so we + // can use it if we import it. The import // `use forge_app::dto::google::{EventData, Request};` is already there. let request = Request::from(context); diff --git a/crates/forge_repo/src/provider/openai.rs b/crates/forge_repo/src/provider/openai.rs index 9f262e98ed..ad189181d5 100644 --- a/crates/forge_repo/src/provider/openai.rs +++ b/crates/forge_repo/src/provider/openai.rs @@ -144,15 +144,17 @@ impl<H: HttpInfra> OpenAIProvider<H> { ); } - // Add GitHub Copilot optimization headers only for github_copilot provider + // Add GitHub Copilot optimization headers only for github_copilot + // provider if self.provider.id == ProviderId::GITHUB_COPILOT { - // Determine initiator: use request.initiator if available, otherwise detect - // from messages + // Determine initiator: use request.initiator if available, + // otherwise detect from messages let initiator = request.initiator.as_deref().unwrap_or_else(|| { // Fall back to detecting from last message role let is_agent_initiated = request.messages.as_ref().is_some_and(|messages| { messages.last().is_some_and(|msg| { - // If last message role is not User, it's agent-initiated + // If last message role is not User, it's + // agent-initiated !matches!(msg.role, forge_app::dto::openai::Role::User) }) }); @@ -183,7 +185,8 @@ impl<H: HttpInfra> OpenAIProvider<H> { headers.push(("Copilot-Vision-Request".to_string(), "true".to_string())); } - // When Copilot proxies an Anthropic Claude model, inject the beta flag + // When Copilot proxies an Anthropic Claude model, inject the beta + // flag let is_anthropic_model = request .model .as_ref() @@ -251,7 +254,8 @@ impl<H: HttpInfra> OpenAIProvider<H> { } async fn inner_models(&self) -> Result<Vec<forge_app::domain::Model>> { - // For Vertex AI, load models from static JSON file using VertexProvider logic + // For Vertex AI, load models from static JSON file using VertexProvider + // logic if self.provider.id == ProviderId::VERTEX_AI { debug!("Loading Vertex AI models from static JSON file"); Ok(self.inner_vertex_models()) @@ -270,10 +274,12 @@ impl<H: HttpInfra> OpenAIProvider<H> { anyhow::bail!(error) } Ok(response) => { - // GitHub Copilot's /models endpoint uses a different schema - // (capabilities, limits, policy) than the standard OpenAI - // models response and includes models the account cannot - // actually use + // GitHub Copilot's /models endpoint uses a + // different schema + // (capabilities, limits, policy) than the standard + // OpenAI + // models response and includes models the account + // cannot actually use if self.provider.id == ProviderId::GITHUB_COPILOT { let data: CopilotListModelResponse = serde_json::from_str( &response, @@ -282,8 +288,10 @@ impl<H: HttpInfra> OpenAIProvider<H> { .with_context( || "Failed to deserialize GitHub Copilot models response", )?; - // Offer the synthetic "auto" model first; it lets the - // service pick the model and is the included option on + // Offer the synthetic "auto" model first; it + // lets the + // service pick the model and is the included + // option on // plans with limited premium requests let models = std::iter::once(copilot_auto_model()) .chain( @@ -787,7 +795,8 @@ mod tests { let headers = openai_provider.get_headers_with_request(&request); - // Should only have Authorization header (no Session-Id for non-zai providers) + // Should only have Authorization header (no Session-Id for non-zai + // providers) assert_eq!(headers.len(), 1); assert!( headers @@ -809,7 +818,8 @@ mod tests { let headers = openai_provider.get_headers_with_request(&request); - // Should only have Authorization header (no Session-Id when session_id is None) + // Should only have Authorization header (no Session-Id when session_id + // is None) assert_eq!(headers.len(), 1); assert!( headers @@ -834,7 +844,8 @@ mod tests { let headers = openai_provider.get_headers_with_request(&request); - // Should only have Authorization header (no Session-Id for Anthropic providers) + // Should only have Authorization header (no Session-Id for Anthropic + // providers) assert_eq!(headers.len(), 1); assert!( headers @@ -994,7 +1005,8 @@ mod tests { let headers = openai_provider.get_headers_with_request(&request); - // Should have Authorization, x-initiator (user), and Openai-Intent headers + // Should have Authorization, x-initiator (user), and Openai-Intent + // headers assert!( headers .iter() @@ -1131,7 +1143,8 @@ mod tests { #[tokio::test] async fn test_get_headers_with_request_non_github_copilot_no_extra_headers() -> anyhow::Result<()> { - // Verify that non-GitHub Copilot providers don't get the optimization headers + // Verify that non-GitHub Copilot providers don't get the optimization + // headers let provider = openai("test-key"); let http_client = Arc::new(MockHttpClient::new()); let openai_provider = OpenAIProvider::new(provider, http_client); diff --git a/crates/forge_repo/src/provider/openai_responses/repository.rs b/crates/forge_repo/src/provider/openai_responses/repository.rs index 847e570c73..2cd34cbed4 100644 --- a/crates/forge_repo/src/provider/openai_responses/repository.rs +++ b/crates/forge_repo/src/provider/openai_responses/repository.rs @@ -164,7 +164,8 @@ impl<T: HttpInfra> OpenAIResponsesProvider<T> { let mut request = oai::CreateResponse::from_domain(context)?; request.model = Some(model.as_str().to_string()); - // Apply Codex-specific request adjustments via the transformer pipeline. + // Apply Codex-specific request adjustments via the transformer + // pipeline. if self.provider.id == forge_domain::ProviderId::CODEX { use forge_domain::Transformer; request = super::codex_transformer::CodexTransformer.transform(request); diff --git a/crates/forge_repo/src/provider/openai_responses/request.rs b/crates/forge_repo/src/provider/openai_responses/request.rs index 219094a8c3..b91fb2c7b1 100644 --- a/crates/forge_repo/src/provider/openai_responses/request.rs +++ b/crates/forge_repo/src/provider/openai_responses/request.rs @@ -142,8 +142,9 @@ impl FromDomain<ReasoningConfig> for oai::Reasoning { builder.summary(oai::ReasoningSummary::Auto); } - // Note: max_tokens is not supported in the OpenAI Responses API's ReasoningArgs - // It's controlled at the request level via max_output_tokens + // Note: max_tokens is not supported in the OpenAI Responses API's + // ReasoningArgs It's controlled at the request level via + // max_output_tokens builder.build().map_err(anyhow::Error::from) } @@ -293,8 +294,9 @@ impl FromDomain<ChatContext> for oai::CreateResponse { ))); } ContextMessage::Image(img) => { - // Mirror the Chat Completions request path: represent image input - // as a user message with structured content. + // Mirror the Chat Completions request path: represent image + // input as a user message with + // structured content. items.push(oai::InputItem::EasyMessage(oai::EasyInputMessage { r#type: oai::MessageType::Message, role: oai::Role::User, @@ -356,9 +358,9 @@ impl FromDomain<ChatContext> for oai::CreateResponse { builder.temperature(temperature.value()); } - // Some OpenAI Codex/"reasoning" models reject `top_p` entirely (even when set - // to defaults). To avoid hard failures, we currently omit it for the - // Responses API path. + // Some OpenAI Codex/"reasoning" models reject `top_p` entirely (even + // when set to defaults). To avoid hard failures, we currently + // omit it for the Responses API path. if let Some(tools) = tools { builder.tools(tools); @@ -1413,8 +1415,8 @@ mod tests { anyhow::bail!("Expected items input"); }; - // Should only have user message, function call, and function call output - // Empty assistant message should be skipped + // Should only have user message, function call, and function call + // output Empty assistant message should be skipped assert_eq!(items.len(), 3); Ok(()) diff --git a/crates/forge_repo/src/provider/openai_responses/response.rs b/crates/forge_repo/src/provider/openai_responses/response.rs index 9e91097a84..4a17fb6bea 100644 --- a/crates/forge_repo/src/provider/openai_responses/response.rs +++ b/crates/forge_repo/src/provider/openai_responses/response.rs @@ -215,9 +215,10 @@ impl IntoDomain for oai::Response { } } - // Process reasoning summary - include the reasoning id so that - // summary parts can be grouped with their encrypted counterpart - // when replayed back to the API. + // Process reasoning summary - include the reasoning id so + // that summary parts can be grouped + // with their encrypted counterpart when + // replayed back to the API. if !reasoning.summary.is_empty() { let mut summary_texts = Vec::new(); for summary_part in &reasoning.summary { @@ -399,8 +400,10 @@ impl IntoDomain for BoxStream<StreamItem, anyhow::Error> { (tool_call_id.clone(), tool_name.clone()), ); - // Only emit if we have non-empty initial arguments. - // Otherwise, wait for deltas or done event. + // Only emit if we have non-empty + // initial arguments. + // Otherwise, wait for deltas or done + // event. if !call.arguments.is_empty() { Some(Ok(ChatCompletionMessage::default() .add_tool_call(ToolCall::Part(ToolCallPart { @@ -414,7 +417,8 @@ impl IntoDomain for BoxStream<StreamItem, anyhow::Error> { } } oai::OutputItem::Reasoning(_reasoning) => { - // Reasoning items don't emit content in real-time, only at + // Reasoning items don't emit content in + // real-time, only at // completion None } @@ -451,16 +455,20 @@ impl IntoDomain for BoxStream<StreamItem, anyhow::Error> { ))) } oai::ResponseStreamEvent::ResponseFunctionCallArgumentsDone(done) => { - // If deltas were already streamed for this output index, - // the arguments have already been emitted incrementally. + // If deltas were already streamed for this + // output index, + // the arguments have already been emitted + // incrementally. if state .received_toolcall_deltas .contains(&(done.output_index.into())) { None } else { - // No deltas were received (e.g. the Spark model sends - // the complete arguments only in the `done` event). + // No deltas were received (e.g. the Spark + // model sends + // the complete arguments only in the `done` + // event). // Emit the full tool call now. let (call_id, name) = state .output_index_to_tool_call @@ -491,16 +499,19 @@ impl IntoDomain for BoxStream<StreamItem, anyhow::Error> { } } oai::ResponseStreamEvent::ResponseCompleted(done) => { - // Text content, reasoning, and tool calls were already streamed via + // Text content, reasoning, and tool calls were + // already streamed via // delta events Only emit metadata // (usage, finish_reason) let mut message: ChatCompletionMessage = done.response.into_domain(); message.content = None; // Clear content to avoid duplication message.reasoning = None; // Clear reasoning to avoid duplication - // Keep only encrypted-content reasoning details — text and + // Keep only encrypted-content reasoning details + // — text and // summary were already streamed via deltas but - // encrypted_content is never streamed and must be preserved + // encrypted_content is never streamed and must + // be preserved // for multi-turn reasoning replay. message.reasoning_details = retain_encrypted_reasoning_details(message.reasoning_details); @@ -508,13 +519,15 @@ impl IntoDomain for BoxStream<StreamItem, anyhow::Error> { Some(Ok(message)) } oai::ResponseStreamEvent::ResponseIncomplete(done) => { - // Text content, reasoning, and tool calls were already streamed via + // Text content, reasoning, and tool calls were + // already streamed via // delta events let mut message: ChatCompletionMessage = done.response.into_domain(); message.content = None; // Clear content to avoid duplication message.reasoning = None; // Clear reasoning to avoid duplication - // Keep only encrypted-content reasoning details (see above). + // Keep only encrypted-content reasoning details + // (see above). message.reasoning_details = retain_encrypted_reasoning_details(message.reasoning_details); message.tool_calls.clear(); // Clear tool calls to avoid duplication @@ -1299,7 +1312,8 @@ mod tests { #[tokio::test] async fn test_stream_with_function_call_arguments_done_no_deltas() -> anyhow::Result<()> { - // When no deltas were received, the done event should emit the tool call + // When no deltas were received, the done event should emit the tool + // call let done = oai::ResponseFunctionCallArgumentsDoneEvent { sequence_number: 1, output_index: 0, @@ -1596,7 +1610,8 @@ mod tests { let delta3_msg = messages[2].as_ref().unwrap(); assert_eq!(delta3_msg.content, Some(Content::part("</commit_message>"))); - // Completion event should have NO content (cleared to avoid duplication) + // Completion event should have NO content (cleared to avoid + // duplication) let completion_msg = messages[3].as_ref().unwrap(); assert_eq!(completion_msg.content, None); assert_eq!(completion_msg.finish_reason, Some(FinishReason::Stop)); @@ -1606,12 +1621,14 @@ mod tests { #[tokio::test] async fn test_stream_avoids_duplicate_reasoning_in_completion() -> anyhow::Result<()> { - // Simulate realistic streaming: reasoning deltas followed by completion event + // Simulate realistic streaming: reasoning deltas followed by completion + // event let reasoning_delta1 = fixture_delta_reasoning_text("Analyzing the request..."); let reasoning_delta2 = fixture_delta_reasoning_text(" and formulating response."); let summary_delta = fixture_delta_reasoning_summary("Summary of analysis"); - // Completion event contains the full reasoning that was already streamed + // Completion event contains the full reasoning that was already + // streamed let response = fixture_response_with_reasoning_both( "Analyzing the request... and formulating response.", "Summary of analysis", @@ -1663,8 +1680,8 @@ mod tests { ); assert!(summary_msg.reasoning_details.is_some()); - // Completion event should have NO reasoning or reasoning_details (cleared to - // avoid duplication) + // Completion event should have NO reasoning or reasoning_details + // (cleared to avoid duplication) let completion_msg = messages[3].as_ref().unwrap(); assert_eq!(completion_msg.reasoning, None); assert_eq!(completion_msg.reasoning_details, None); @@ -1675,12 +1692,14 @@ mod tests { #[tokio::test] async fn test_stream_avoids_duplicate_tool_calls_in_completion() -> anyhow::Result<()> { - // Simulate realistic streaming: tool call deltas followed by completion event + // Simulate realistic streaming: tool call deltas followed by completion + // event let added = fixture_function_call_added("call_123", "shell", ""); let delta1 = fixture_function_call_arguments_delta(0, r#"{"cmd":"echo"#); let delta2 = fixture_function_call_arguments_delta(0, r#" hello"}"#); - // Completion event contains the full tool call that was already streamed + // Completion event contains the full tool call that was already + // streamed let response = fixture_response_with_function_call("call_123", "shell", r#"{"cmd":"echo hello"}"#); let completed = oai::ResponseCompletedEvent { sequence_number: 4, response }; @@ -1709,7 +1728,8 @@ mod tests { let delta2_msg = messages[1].as_ref().unwrap(); assert_eq!(delta2_msg.tool_calls.len(), 1); - // Completion event should have NO tool calls (cleared to avoid duplication) + // Completion event should have NO tool calls (cleared to avoid + // duplication) let completion_msg = messages[2].as_ref().unwrap(); assert_eq!(completion_msg.tool_calls.len(), 0); assert_eq!(completion_msg.finish_reason, Some(FinishReason::ToolCalls)); @@ -1824,7 +1844,8 @@ mod tests { /// 3. response.completed #[tokio::test] async fn test_spark_style_stream_function_call_no_deltas() -> anyhow::Result<()> { - // Step 1: output_item.added with empty arguments (Spark sends "" initially) + // Step 1: output_item.added with empty arguments (Spark sends "" + // initially) let added = fixture_function_call_added("call_shkZ0WZ4bgS2HdaAF0YOcB06", "shell", ""); // Step 2: function_call_arguments.done with full arguments (no deltas) diff --git a/crates/forge_repo/src/provider/provider_repo.rs b/crates/forge_repo/src/provider/provider_repo.rs index c8bb120b7d..7f4329315a 100644 --- a/crates/forge_repo/src/provider/provider_repo.rs +++ b/crates/forge_repo/src/provider/provider_repo.rs @@ -309,7 +309,8 @@ impl< continue; } - // Try to create configured template provider, fallback to unconfigured + // Try to create configured template provider, fallback to + // unconfigured let provider_entry = if let Ok(provider) = self.create_provider(&config).await { Some(provider.into()) } else if let Ok(provider) = self.create_unconfigured_provider(&config) { @@ -349,8 +350,8 @@ impl< let has_anthropic_url = self.infra.get_env_var("ANTHROPIC_URL").is_some(); for config in configs { - // Skip Forge provider and ContextEngine providers - they're not configurable - // via env like other providers + // Skip Forge provider and ContextEngine providers - they're not + // configurable via env like other providers if config.id == ProviderId::FORGE || config.provider_type == ProviderType::ContextEngine { continue; @@ -537,7 +538,8 @@ impl< access_token.token.chars().take(20).collect::<String>() ); - // Create new credential with fresh token, preserving url_params and provider ID + // Create new credential with fresh token, preserving url_params and + // provider ID Ok(forge_domain::AuthCredential::new_api_key( original_credential.id.clone(), forge_domain::ApiKey::from(access_token.token), @@ -551,11 +553,13 @@ impl< ) -> anyhow::Result<forge_domain::ProviderTemplate> { // Handle special cases first if id == ProviderId::FORGE { - // Forge provider isn't typically configured via env vars in the registry + // Forge provider isn't typically configured via env vars in the + // registry return Err(Error::provider_not_available(ProviderId::FORGE).into()); } - // Look up provider from cached providers - return configured template providers + // Look up provider from cached providers - return configured template + // providers self.get_providers() .await .iter() @@ -1282,8 +1286,8 @@ mod env_tests { #[async_trait::async_trait] impl FileWriterInfra for MockInfra { async fn write(&self, path: &std::path::Path, content: Bytes) -> anyhow::Result<()> { - // Capture writes to credentials file and persist to the real temp dir - // so that OS-level permission checks work in tests. + // Capture writes to credentials file and persist to the real temp + // dir so that OS-level permission checks work in tests. if path == self.get_environment().credentials_path() { let content_str = String::from_utf8(content.to_vec())?; let creds: Vec<AuthCredential> = serde_json::from_str(&content_str)?; @@ -1747,7 +1751,8 @@ mod env_tests { }) .unwrap(); - // Regular OpenAI and Anthropic providers return template URLs (not rendered) + // Regular OpenAI and Anthropic providers return template URLs (not + // rendered) assert_eq!( openai_provider.url.template, "https://api.openai.com/v1/chat/completions" diff --git a/crates/forge_repo/src/skill.rs b/crates/forge_repo/src/skill.rs index 1b598b0fb6..75aa37b41d 100644 --- a/crates/forge_repo/src/skill.rs +++ b/crates/forge_repo/src/skill.rs @@ -99,8 +99,8 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> SkillR let cwd_skills = self.load_skills_from_dir(&cwd_dir).await?; skills.extend(cwd_skills); - // Resolve conflicts by keeping the last occurrence (CWD > Agents > Global > - // Built-in) + // Resolve conflicts by keeping the last occurrence (CWD > Agents > + // Global > Built-in) let skills = resolve_skill_conflicts(skills); // Render all skills with environment context @@ -163,7 +163,8 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> ForgeS .unwrap_or("unknown") .to_string(); - // Get all resource files in the skill directory recursively + // Get all resource files in the skill directory + // recursively let walker = Walker::unlimited().cwd(subdir.clone()); let mut resources = infra .walk(walker) @@ -171,7 +172,8 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> ForgeS .unwrap_or_default() .into_iter() .filter_map(|walked| { - // Only include files (not directories) and exclude SKILL.md + // Only include files (not directories) and + // exclude SKILL.md if !walked.is_dir() { let full_path = subdir.join(&walked.path); if full_path.file_name() != skill_path.file_name() { @@ -186,12 +188,14 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> ForgeS .collect::<Vec<_>>(); sort_paths(&mut resources); - // Try to extract skill from front matter, otherwise create with - // directory name + // Try to extract skill from front matter, otherwise + // create with directory + // name if let Some(skill) = extract_skill(&path_str, &content) { Ok(Some(skill.resources(resources))) } else { - // Fallback: create skill with directory name if front matter is + // Fallback: create skill with directory name if + // front matter is // missing Ok(Some( Skill::new(skill_name, content, String::new()) diff --git a/crates/forge_select/src/input.rs b/crates/forge_select/src/input.rs index 515893c8fe..e494e8935b 100644 --- a/crates/forge_select/src/input.rs +++ b/crates/forge_select/src/input.rs @@ -74,8 +74,9 @@ impl InputBuilder { let mut rl = DefaultEditor::new()?; - // On Windows, rustyline miscounts ANSI escape bytes as visible characters, - // causing incorrect cursor placement and extra space before the editor. + // On Windows, rustyline miscounts ANSI escape bytes as visible + // characters, causing incorrect cursor placement and extra + // space before the editor. let prompt_str = if cfg!(windows) { format!("? {}: ", self.message) } else { diff --git a/crates/forge_services/src/agent_registry.rs b/crates/forge_services/src/agent_registry.rs index 5e547bd212..b381a2c3f5 100644 --- a/crates/forge_services/src/agent_registry.rs +++ b/crates/forge_services/src/agent_registry.rs @@ -51,8 +51,8 @@ impl<R: AgentRepository + EnvironmentInfra<Config = forge_config::ForgeConfig>> // Not loaded yet, acquire write lock and load let mut agents_write = self.agents.write().await; - // Double-check in case another task loaded while we were waiting for write - // lock + // Double-check in case another task loaded while we were waiting for + // write lock if let Some(agents) = agents_write.as_ref() { return Ok(agents.clone()); } diff --git a/crates/forge_services/src/app_config.rs b/crates/forge_services/src/app_config.rs index 3e279aae9b..8a0b259c8e 100644 --- a/crates/forge_services/src/app_config.rs +++ b/crates/forge_services/src/app_config.rs @@ -345,7 +345,8 @@ mod tests { .await?; // Should return the config even if provider is not available - // Validation happens when getting the actual provider via ProviderService + // Validation happens when getting the actual provider via + // ProviderService let result = service.get_session_config().await; assert_eq!( diff --git a/crates/forge_services/src/attachment.rs b/crates/forge_services/src/attachment.rs index 5ebfe6bf29..b1d695776c 100644 --- a/crates/forge_services/src/attachment.rs +++ b/crates/forge_services/src/attachment.rs @@ -43,8 +43,8 @@ impl< // Check if path is a directory (exists but is not a file) if self.infra.exists(&path).await? && !self.infra.is_file(&path).await? { - // List all entries (files and directories) efficiently without reading file - // contents + // List all entries (files and directories) efficiently without + // reading file contents let dir_entries = self.infra.list_directory_entries(&path).await?; // Create DirectoryEntry for each entry @@ -290,7 +290,8 @@ pub mod tests { start_idx as u64 + filtered_lines.len() as u64 }; - // Compute hash from the full file content to match production behaviour + // Compute hash from the full file content to match production + // behaviour let content_hash = compute_hash(&full_content); Ok(( @@ -586,7 +587,8 @@ pub mod tests { let attachment = attachments.first().unwrap(); assert_eq!(attachment.path, "/test/file1.txt"); - // Check that the content contains our original text and has range information + // Check that the content contains our original text and has range + // information assert!(attachment.content.contains("This is a text file content")); } @@ -607,8 +609,8 @@ pub mod tests { let attachment = attachments.first().unwrap(); assert_eq!(attachment.path, "/test/image.png"); - // Base64 content should be the encoded mock binary content with proper data URI - // format + // Base64 content should be the encoded mock binary content with proper + // data URI format let expected_base64 = base64::engine::general_purpose::STANDARD.encode("mock-binary-content"); assert_eq!( @@ -634,8 +636,8 @@ pub mod tests { let attachment = attachments.first().unwrap(); assert_eq!(attachment.path, "/test/image with spaces.jpg"); - // Base64 content should be the encoded mock jpeg content with proper data URI - // format + // Base64 content should be the encoded mock jpeg content with proper + // data URI format let expected_base64 = base64::engine::general_purpose::STANDARD.encode("mock-jpeg-content"); assert_eq!( attachment.content.as_image().unwrap().url().as_str(), @@ -741,7 +743,8 @@ pub mod tests { let attachment = attachments.first().unwrap(); assert_eq!(attachment.path, "/test/unknown.xyz"); - // Check that the content contains our original text and has range information + // Check that the content contains our original text and has range + // information assert!(attachment.content.contains("Some content")); } @@ -1101,7 +1104,8 @@ pub mod tests { let url = "@[/test/emptydir]"; let attachments = chat_request.attachments(url).await.unwrap(); - // Should return a single DirectoryListing attachment with empty files list + // Should return a single DirectoryListing attachment with empty files + // list assert_eq!(attachments.len(), 1); let attachment = attachments.first().unwrap(); diff --git a/crates/forge_services/src/clipper.rs b/crates/forge_services/src/clipper.rs index 7770b1ed4e..39eff5e246 100644 --- a/crates/forge_services/src/clipper.rs +++ b/crates/forge_services/src/clipper.rs @@ -159,8 +159,8 @@ impl Clipper { prefix_limit: usize, suffix_limit: usize, ) -> ClipperResult<'a> { - // If the combined limits exceed or equal content length, return the whole - // content + // If the combined limits exceed or equal content length, return the + // whole content if prefix_limit + suffix_limit >= char_count { return ClipperResult { prefix: None, suffix: None, actual: content }; } @@ -253,8 +253,8 @@ mod tests { let result = strategy.clip(content); - // Should return the original content as the combined limits exceed content - // length + // Should return the original content as the combined limits exceed + // content length assert!(result.prefix.is_none()); assert!(result.suffix.is_none()); assert_eq!(result.actual, content); diff --git a/crates/forge_services/src/command.rs b/crates/forge_services/src/command.rs index 8155bda643..6c3aa3e2c4 100644 --- a/crates/forge_services/src/command.rs +++ b/crates/forge_services/src/command.rs @@ -108,8 +108,8 @@ fn resolve_command_conflicts(commands: Vec<Command>) -> Vec<Command> { command_map.insert(command.name.clone(), command); } - // Convert back to vector (order is not guaranteed but doesn't matter for the - // service) + // Convert back to vector (order is not guaranteed but doesn't matter for + // the service) command_map.into_values().collect() } @@ -263,8 +263,8 @@ mod tests { let actual = resolve_command_conflicts(fixture); - // Should have 3 commands: command1 (CWD version), command2 (global), command3 - // (CWD) + // Should have 3 commands: command1 (CWD version), command2 (global), + // command3 (CWD) assert_eq!(actual.len(), 3); let command1 = actual @@ -301,7 +301,8 @@ mod tests { let actual = resolve_command_conflicts(fixture); - // Should have 4 commands: common (CWD version), unique1, unique2, unique3 + // Should have 4 commands: common (CWD version), unique1, unique2, + // unique3 assert_eq!(actual.len(), 4); let common = actual diff --git a/crates/forge_services/src/context_engine.rs b/crates/forge_services/src/context_engine.rs index 713ca56bb4..77e89e6d75 100644 --- a/crates/forge_services/src/context_engine.rs +++ b/crates/forge_services/src/context_engine.rs @@ -313,7 +313,8 @@ impl< /// Deletes multiple workspaces in parallel from both the server and local /// database. async fn delete_workspaces(&self, workspace_ids: &[forge_domain::WorkspaceId]) -> Result<()> { - // Delete all workspaces in parallel by calling delete_workspace for each + // Delete all workspaces in parallel by calling delete_workspace for + // each let delete_tasks: Vec<_> = workspace_ids .iter() .map(|workspace_id| self.delete_workspace(workspace_id)) @@ -355,8 +356,8 @@ impl< let workspace = self.get_workspace_by_path(path, &token).await?; - // Reuse the canonical path already stored in the workspace (resolved during - // sync), avoiding a redundant canonicalize() IO call. + // Reuse the canonical path already stored in the workspace (resolved + // during sync), avoiding a redundant canonicalize() IO call. let canonical_path = PathBuf::from(&workspace.working_dir); let batch_size = self.infra.get_config()?.max_file_read_batch_size; diff --git a/crates/forge_services/src/fd.rs b/crates/forge_services/src/fd.rs index a86e3ac171..749432f2ce 100644 --- a/crates/forge_services/src/fd.rs +++ b/crates/forge_services/src/fd.rs @@ -40,19 +40,13 @@ fn is_ignored_by_name(path: &Path) -> bool { let name_lower = name.to_lowercase(); // Lock files: *-lock.json, *.lock, *.lockb, *.lock.json, etc. - if name_lower.ends_with(".lock") + name_lower.ends_with(".lock") || name_lower.ends_with(".lockb") || name_lower.ends_with("-lock.json") || name_lower.ends_with("-lock.yaml") || name_lower.ends_with("-lock.yml") || name_lower.ends_with(".lock.json") - || name_lower.ends_with(".lockfile") - || name == "Package.resolved" - { - return true; - } - - false + || name_lower.ends_with(".lockfile") || name == "Package.resolved" } /// Returns `true` if `path` is a symlink (does not follow the link). @@ -176,7 +170,8 @@ mod tests { .write_all(b"fn main() {}") .unwrap(); - // Symlink pointing to the real file (also carries an allowed extension). + // Symlink pointing to the real file (also carries an allowed + // extension). let link_path = base.join("link.rs"); std::os::unix::fs::symlink(&real_path, &link_path).unwrap(); diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index ad6c92e6be..ef34ee062b 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -175,7 +175,8 @@ where .await?; // Clear the unified cache to force refresh on next use - // Since we now use a merged hash, clearing any scope invalidates the cache + // Since we now use a merged hash, clearing any scope invalidates the + // cache self.infra.cache_clear().await?; Ok(()) @@ -201,7 +202,8 @@ where McpConfig::default() }; - // Merge: user first, then local (local takes precedence as in read_mcp_config). + // Merge: user first, then local (local takes precedence as in + // read_mcp_config). let mut merged = user_config; merged.merge(local_config); diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index 4b848e2b01..6b67b0fd0e 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -103,17 +103,18 @@ where async fn ensure_mcp_initialized(&self) -> anyhow::Result<()> { let raw_mcp = self.manager.read_mcp_config(None).await?; - // Fast path: if config is unchanged, skip reinitialization without acquiring - // the lock + // Fast path: if config is unchanged, skip reinitialization without + // acquiring the lock if !self.is_config_modified(&raw_mcp).await { return Ok(()); } - // Serialise concurrent initialisations so only one caller runs update_mcp at a - // time + // Serialise concurrent initialisations so only one caller runs + // update_mcp at a time let _guard = self.init_lock.lock().await; - // Double-check under the lock: a concurrent caller may have already updated + // Double-check under the lock: a concurrent caller may have already + // updated if !self.is_config_modified(&raw_mcp).await { return Ok(()); } @@ -127,9 +128,9 @@ where } async fn update_mcp(&self, mcp: McpConfig) -> Result<(), anyhow::Error> { - // Use the raw config hash (pre-trust-gate) so that is_config_modified always - // compares against the original file hash, preventing infinite re-prompt loops - // when some servers are rejected. + // Use the raw config hash (pre-trust-gate) so that is_config_modified + // always compares against the original file hash, preventing + // infinite re-prompt loops when some servers are rejected. let new_hash = mcp.cache_key(); self.clear_tools().await; @@ -157,7 +158,8 @@ where Ok(_) => {} Err(error) => { // Format error with full chain for detailed diagnostics - // Using Debug formatting with alternate flag shows the full error chain + // Using Debug formatting with alternate flag shows the full + // error chain let error_string = format!("{error:?}"); self.failed_servers .write() @@ -203,7 +205,8 @@ where let tools = self.tools.read().await; // Try exact match first, then fall back to legacy-format lookup for - // tool calls arriving in the Claude Code `mcp__{server}__{tool}` format. + // tool calls arriving in the Claude Code `mcp__{server}__{tool}` + // format. let tool = tools .get(&call.name) .or_else(|| call.name.to_legacy_mcp_name().and_then(|n| tools.get(&n))) @@ -221,7 +224,8 @@ where // this, clear_tools could run while connections are still being // established, leaving waiters released into an empty tool map. let _guard = self.init_lock.lock().await; - // Clear the infra cache and reset config hash to force re-init on next access + // Clear the infra cache and reset config hash to force re-init on next + // access self.infra.cache_clear().await?; *self.previous_config_hash.lock().await = Default::default(); self.clear_tools().await; @@ -239,9 +243,10 @@ where { async fn get_mcp_servers(&self) -> anyhow::Result<McpServers> { // Apply the trust gate before computing the cache key so that rejected - // servers are excluded. Using the raw config hash would allow a stale KV - // cache entry (populated before a rejection) to be returned, bypassing - // filter_trusted entirely and leaking rejected tools into requests. + // servers are excluded. Using the raw config hash would allow a stale + // KV cache entry (populated before a rejection) to be returned, + // bypassing filter_trusted entirely and leaking rejected tools + // into requests. let raw_mcp = self.manager.read_mcp_config(None).await?; let trusted_mcp = self.manager.filter_trusted(raw_mcp).await?; let config_hash = trusted_mcp.cache_key(); @@ -265,9 +270,10 @@ where } async fn init_mcp(&self) -> anyhow::Result<()> { - // Run the trust gate prompt at startup so the user's decision is captured - // before any tool use. The result is intentionally discarded — servers are - // NOT connected here. Connections remain lazy and happen on first tool use + // Run the trust gate prompt at startup so the user's decision is + // captured before any tool use. The result is intentionally + // discarded — servers are NOT connected here. Connections + // remain lazy and happen on first tool use // via ensure_mcp_initialized. let raw_mcp = self.manager.read_mcp_config(None).await?; let _ = self.manager.filter_trusted(raw_mcp).await?; diff --git a/crates/forge_services/src/provider_auth.rs b/crates/forge_services/src/provider_auth.rs index 67c27b9595..2fdd0d8710 100644 --- a/crates/forge_services/src/provider_auth.rs +++ b/crates/forge_services/src/provider_auth.rs @@ -55,7 +55,8 @@ where )?; let mut request = strategy.init().await?; - // For API key flow and Google ADC, attach existing credential if available + // For API key flow and Google ADC, attach existing credential if + // available if let AuthContextRequest::ApiKey(ref mut api_key_request) = request && let Ok(Some(existing_credential)) = self.infra.get_credential(&provider_id).await { @@ -85,10 +86,12 @@ where _timeout: Duration, ) -> anyhow::Result<()> { // Extract auth method from context response - // For ApiKey responses, we need to check if it's Google ADC or regular API key + // For ApiKey responses, we need to check if it's Google ADC or regular + // API key let auth_method = match &auth_context_response { AuthContextResponse::ApiKey(response) => { - // Check if provider supports Google ADC and if it's the Google ADC marker + // Check if provider supports Google ADC and if it's the Google + // ADC marker let is_vertex_provider = provider_id == forge_domain::ProviderId::VERTEX_AI || provider_id == forge_domain::ProviderId::VERTEX_AI_ANTHROPIC; if is_vertex_provider && response.response.api_key.as_ref() == "google_adc_marker" { @@ -167,7 +170,8 @@ where }, )?; - // Get required params (only used for API key, but needed for factory) + // Get required params (only used for API key, but + // needed for factory) let required_params = if matches!(auth_method, AuthMethod::ApiKey) { provider.url_params.clone() } else { @@ -192,7 +196,8 @@ where continue; } - // Update provider with refreshed credential + // Update provider with refreshed + // credential provider.credential = Some(refreshed); break; // Success, stop trying other methods } diff --git a/crates/forge_services/src/provider_service.rs b/crates/forge_services/src/provider_service.rs index 5baac55ebb..722d3ffb9a 100644 --- a/crates/forge_services/src/provider_service.rs +++ b/crates/forge_services/src/provider_service.rs @@ -111,7 +111,8 @@ impl<R: ChatRepository + ProviderRepository> ProviderService for ForgeProviderSe context: Context, provider: Provider<Url>, ) -> ResultStream<ChatCompletionMessage, anyhow::Error> { - // Repository builds client on each call (no caching at repository level) + // Repository builds client on each call (no caching at repository + // level) self.repository.chat(model_id, context, provider).await } diff --git a/crates/forge_services/src/template.rs b/crates/forge_services/src/template.rs index 19a63c357a..bab776f83e 100644 --- a/crates/forge_services/src/template.rs +++ b/crates/forge_services/src/template.rs @@ -184,8 +184,8 @@ mod tests { .await .unwrap(); - // Expected: Result should contain the rendered system info with substituted - // values + // Expected: Result should contain the rendered system info with + // substituted values assert!(actual.contains("<operating_system>test-os</operating_system>")); assert!(actual.contains("file1.txt")); assert!(actual.contains("file2.txt")); @@ -204,7 +204,8 @@ mod tests { hb.register_template("test", template); let actual = hb.render("test", &test_data).unwrap(); - // Expected: Should render the handlebars template with substituted values + // Expected: Should render the handlebars template with substituted + // values let expected = "Hello World!"; assert_eq!(actual, expected); } diff --git a/crates/forge_services/src/tool_services/fs_patch.rs b/crates/forge_services/src/tool_services/fs_patch.rs index 7d99ac01b6..e8d1a43298 100644 --- a/crates/forge_services/src/tool_services/fs_patch.rs +++ b/crates/forge_services/src/tool_services/fs_patch.rs @@ -75,8 +75,9 @@ impl Range { // SearchMatch uses 0-based inclusive line numbers // Convert to 0-based array indices let start_idx = (search_match.start_line as usize).min(lines.len()); - // end_line is 0-based inclusive, convert to 0-based exclusive for slicing - // Add 1 to make it exclusive: line 0 to line 0 means [0..1], one line + // end_line is 0-based inclusive, convert to 0-based exclusive for + // slicing Add 1 to make it exclusive: line 0 to line 0 means + // [0..1], one line let end_idx = ((search_match.end_line as usize) + 1).min(lines.len()); // Find the byte position of the start line. @@ -98,8 +99,9 @@ impl Range { lines.get(start_idx).map_or(0, |l| l.len()) } } else { - // Multi-line match: include newlines between lines but NOT after the last line - // Sum lengths of lines from start_idx to end_idx (exclusive) + // Multi-line match: include newlines between lines but NOT after + // the last line Sum lengths of lines from start_idx to + // end_idx (exclusive) let content_len: usize = if start_idx >= lines.len() || end_idx > lines.len() { 0 // Out of bounds match } else { @@ -108,7 +110,8 @@ impl Range { .map_or(0, |slice| slice.iter().map(|l| l.len()).sum()) }; let newlines_between = end_idx - start_idx - 1; - // Count actual newline bytes (\r\n = 2, \n = 1) to handle mixed endings + // Count actual newline bytes (\r\n = 2, \n = 1) to handle mixed + // endings let newline_bytes: usize = source .split('\n') .skip(start_idx) @@ -286,7 +289,8 @@ fn apply_replacement( if (patch.start <= target_patch.start && patch.end() > target_patch.start) || (target_patch.start <= patch.start && target_patch.end() > patch.start) { - // For overlapping ranges, we just do an ordinary replacement + // For overlapping ranges, we just do an ordinary + // replacement let before = haystack.get(..patch.start).ok_or(Error::RangeOutOfBounds( 0, patch.start, @@ -345,7 +349,8 @@ fn apply_replacement( } } } else { - // No match (range is None) - treat as empty search (full file operation) + // No match (range is None) - treat as empty search (full file + // operation) match operation { // Append to the end of the file PatchOperation::Append => Ok(format!("{haystack}{line_ending}{normalized_content}")), diff --git a/crates/forge_services/src/tool_services/fs_read.rs b/crates/forge_services/src/tool_services/fs_read.rs index a5cf78868c..64932b5b40 100644 --- a/crates/forge_services/src/tool_services/fs_read.rs +++ b/crates/forge_services/src/tool_services/fs_read.rs @@ -118,7 +118,8 @@ impl<F: FileInfoInfra + EnvironmentInfra<Config = forge_config::ForgeConfig> + I let config = self.infra.get_config()?; - // Validate with the larger limit initially since we don't know file type yet + // Validate with the larger limit initially since we don't know file + // type yet let initial_size_limit = config.max_file_size_bytes.max(config.max_image_size_bytes); assert_file_size(&*self.infra, path, initial_size_limit).await?; diff --git a/crates/forge_services/src/tool_services/fs_search.rs b/crates/forge_services/src/tool_services/fs_search.rs index efa2c50865..d9a4cf8024 100644 --- a/crates/forge_services/src/tool_services/fs_search.rs +++ b/crates/forge_services/src/tool_services/fs_search.rs @@ -102,8 +102,8 @@ impl<W: WalkerInfra + FileReaderInfra + FileInfoInfra> ForgeFsSearch<W> { params: &FSSearch, ) -> anyhow::Result<Vec<PathBuf>> { // Build type matcher once if file_type is specified (for efficiency) - // Filter out empty strings that may arrive from LLM tool calls with nullable - // parameters + // Filter out empty strings that may arrive from LLM tool calls with + // nullable parameters let types_matcher = if let Some(file_type) = params.file_type.as_deref().filter(|s| !s.is_empty()) { use ignore::types::TypesBuilder; @@ -362,8 +362,8 @@ impl Sink for ContextSink { type Error = std::io::Error; fn matched(&mut self, _searcher: &Searcher, mat: &SinkMatch<'_>) -> Result<bool, Self::Error> { - // If we have a pending match, save it first (it's now complete with all after - // context) + // If we have a pending match, save it first (it's now complete with all + // after context) if let Some((line_num, line)) = self.current_match.take() { self.matches.push(Match { path: self.path.clone(), @@ -384,8 +384,8 @@ impl Sink for ContextSink { self.current_after_context.clear(); } - // Store the current match (before_context is already accumulated, after_context - // will be added via context() calls) + // Store the current match (before_context is already accumulated, + // after_context will be added via context() calls) let line_num = mat.line_number().unwrap_or(0) as usize; let line = mat.bytes().to_str_lossy().trim_end().to_string(); self.current_match = Some((line_num, line)); diff --git a/crates/forge_services/src/tool_services/fs_write.rs b/crates/forge_services/src/tool_services/fs_write.rs index 976cbb750b..2f1edb241d 100644 --- a/crates/forge_services/src/tool_services/fs_write.rs +++ b/crates/forge_services/src/tool_services/fs_write.rs @@ -77,7 +77,8 @@ impl< .with_context(|| format!("File already exists at {}", path.display())); } - // Record the file content before modification and detect its line ending style + // Record the file content before modification and detect its line + // ending style let (old_content, target_line_ending) = if file_exists && overwrite { let existing = self.infra.read_utf8(path).await?; let line_ending = if existing.contains("\r\n") { diff --git a/crates/forge_services/src/tool_services/image_read.rs b/crates/forge_services/src/tool_services/image_read.rs index dce227789b..8acd5ff691 100644 --- a/crates/forge_services/src/tool_services/image_read.rs +++ b/crates/forge_services/src/tool_services/image_read.rs @@ -59,8 +59,8 @@ impl< let max_image_size_bytes = self.infra.get_config()?.max_image_size_bytes; - // Validate file size before reading content using image-specific file size - // limit + // Validate file size before reading content using image-specific file + // size limit crate::tool_services::fs_read::assert_file_size(&*self.infra, path, max_image_size_bytes) .await .with_context( diff --git a/crates/forge_snaps/src/service.rs b/crates/forge_snaps/src/service.rs index 4fa11b53dc..46f656477d 100644 --- a/crates/forge_snaps/src/service.rs +++ b/crates/forge_snaps/src/service.rs @@ -57,7 +57,8 @@ impl SnapshotService { pub async fn undo_snapshot(&self, path: PathBuf) -> Result<()> { let snapshot = Snapshot::create(path.clone())?; - // All the snaps for `path` are stored in `snapshot.path_hash()` directory. + // All the snaps for `path` are stored in `snapshot.path_hash()` + // directory. let snapshot_dir = self.snapshots_directory.join(snapshot.path_hash()); // Check if the `snapshot_dir` exists diff --git a/crates/forge_tracker/src/collect/posthog.rs b/crates/forge_tracker/src/collect/posthog.rs index d7c3213a29..84e6efcc74 100644 --- a/crates/forge_tracker/src/collect/posthog.rs +++ b/crates/forge_tracker/src/collect/posthog.rs @@ -18,7 +18,8 @@ pub struct Tracker { impl Tracker { pub fn new(api_secret: &'static str) -> Self { - // Configure HTTP client with connection pooling similar to forge_provider + // Configure HTTP client with connection pooling similar to + // forge_provider let client = Client::builder() .connect_timeout(Duration::from_secs(10)) .read_timeout(Duration::from_secs(30)) diff --git a/crates/forge_walker/src/walker.rs b/crates/forge_walker/src/walker.rs index 4f42fff21a..61c831c52e 100644 --- a/crates/forge_walker/src/walker.rs +++ b/crates/forge_walker/src/walker.rs @@ -118,7 +118,8 @@ impl Walker { pub fn get_blocking(&self) -> Result<Vec<File>> { // Shared state collected across parallel walker threads. let collected: Arc<Mutex<Vec<File>>> = Arc::new(Mutex::new(Vec::new())); - // Per-directory entry counters for breadth limiting (shared across threads). + // Per-directory entry counters for breadth limiting (shared across + // threads). let dir_entries: Arc<Mutex<HashMap<String, usize>>> = Arc::new(Mutex::new(HashMap::new())); // Global counters protected by a single mutex to enforce total limits. // Layout: (total_size, file_count, quit) @@ -141,7 +142,8 @@ impl Walker { // Skip files that exceed size limit .max_filesize(Some(self.max_file_size)) .filter_entry(|entry| { - // Always exclude the `.git` directory, matching `fd --exclude .git`. + // Always exclude the `.git` directory, matching `fd --exclude + // .git`. entry.file_name() != ".git" }) .build_parallel(); @@ -253,8 +255,9 @@ impl Walker { path_string }; - // Filter out entries whose file_size exceeds the per-file limit. - // (WalkBuilder::max_filesize only applies to regular files; double-check.) + // Filter out entries whose file_size exceeds the per-file + // limit. (WalkBuilder::max_filesize only + // applies to regular files; double-check.) if !is_dir && file_size > max_file_size { return ignore::WalkState::Continue; } @@ -520,8 +523,8 @@ mod tests { .await .unwrap(); - // .ignore itself is a dotfile and is visible when hidden: false (matches fd - // --hidden). + // .ignore itself is a dotfile and is visible when hidden: false + // (matches fd --hidden). let mut expected = vec![".ignore", "included/main.rs", "included/test.rs", "base.rs"]; expected.sort(); @@ -656,8 +659,8 @@ mod tests { .map(|f| f.path.as_str()) .collect(); actual.sort(); - // .gitignore files are dotfiles and visible when hidden: false (matches fd - // --hidden). + // .gitignore files are dotfiles and visible when hidden: false (matches + // fd --hidden). let expected = vec![ ".gitignore", "frontend/.gitignore", @@ -701,9 +704,9 @@ mod tests { .map(|f| f.path.as_str()) .collect(); actual.sort(); - // .gitignore files are dotfiles and visible when hidden: false (matches fd - // --hidden). .git directory is always excluded (matching fd --exclude - // .git). + // .gitignore files are dotfiles and visible when hidden: false (matches + // fd --hidden). .git directory is always excluded (matching fd + // --exclude .git). let expected = vec![ ".gitignore", "frontend/.gitignore",