fix: treat empty string as unset for chat.defaultAgent setting #4
Conversation
…generate (aws#2741) * fixing bugs * formatting * fix: CTRL+C handling during multi-select, auto completion for /agent generate * set use legacy mcp config to false --------- Co-authored-by: Xian Wu <xianwwu@amazon.com>
- Changed from ~/.q/knowledge_bases/ to ~/.aws/amazonq/knowledge_bases/ - Default agent uses q_cli_default/ (no alphanumeric suffix) - Custom agents use <agent-name>_<alphanumeric-code>/ format Co-authored-by: Kenneth S. <kennvene@amazon.com>
- Add Drop trait to InputSource for automatic history saving - Replace DefaultHistory with FileHistory for persistence - Store history in ~/.aws/amazonq/cli_history - Refactor ChatHinter to use rustyline's built-in history search - Remove manual history tracking in favor of rustyline's implementation - Add history loading on startup with error handling - Clean up unused hinter history update methods
* comment profile set * comment profile in apiclient * add a helper func * fix compile issue * remove dead code tag
* client struct definition * clean up unused code * adds mechanism for checking if server is alive * prefetches prompts if applicable * fixes agent swap fixes agent swap * only applies process group leader promo for unix * removes unused import for windows * renames abstractions for different stages of mcp config
- Add file://AGENTS.md to default resources list alongside AmazonQ.md - Update test to include both AmazonQ.md and AGENTS.md files - Ensures AGENTS.md is included everywhere AmazonQ.md was previously included Co-authored-by: Matt Lee <mr-lee@users.noreply.github.com>
- Add optional 'model' field to Agent struct for specifying model per agent - Update JSON schema and documentation with model field usage - Integrate agent model into model selection priority: 1. CLI argument (--model) 2. Agent's model field (new) 3. User's saved default model 4. System default model - Add proper fallback when agent specifies unavailable model - Extract fallback logic to eliminate code duplication - Include comprehensive unit tests for model field functionality - Maintain backward compatibility with existing agent configurations Co-authored-by: Matt Lee <mr-lee@users.noreply.github.com>
* Properly handle path with trailing slash in file matching Today if a path has a trailing slash, the glob pattern will look like "/path-to-folder//**" (note the double slash). Glob doesn't work with double slash actually (it doesn't match anything). As a result, the permission management for fs_read and fs_write is broken when allowed or denied path has trailing slash. The fix is to just manually remove the trailing slash. * format change
* add a wrapmode in chat args * add a ut for the wrap arg
- Add auto_allow_readonly field to use_aws Settings struct (defaults to false) - Update eval_perm method to use auto_allow_readonly setting instead of hardcoded behavior - Default behavior: all AWS operations require user confirmation (secure by default) - Opt-in behavior: when autoAllowReadonly=true, read-only operations are auto-approved - Add comprehensive tests covering all scenarios - Maintains backward compatibility through configuration 🤖 Assisted by Amazon Q Developer Co-authored-by: Matt Lee <mr-lee@users.noreply.github.com>
…s#2838) Users can now keep the final question and answer from tangent mode by using `/tangent tail` instead of `/tangent`. This preserves the last Q&A pair when returning to the main conversation, making it easy to retain helpful insights discovered during exploration. - `/tangent` - exits tangent mode (existing behavior unchanged) - `/tangent tail` - exits tangent mode but keeps the last Q&A pair This enables users to safely explore topics without losing the final valuable insight that could benefit their main conversation flow.
Tracks daily active users by sending amazonqcli_dailyHeartbeat event once per day. Uses fail-closed logic to prevent spam during database errors.
* docs: fix local agent directory path - Fix local agent path from .aws/amazonq/cli-agents/ to .amazonq/cli-agents/ - Global paths (~/.aws/amazonq/cli-agents/) remain correct - Aligns documentation with source code implementation * fix: correct workspace agent path in /agent help message The help message for the /agent command incorrectly showed the workspace agent path as 'cwd/.aws/amazonq/cli-agents' when it should be 'cwd/.amazonq/cli-agents' (without the .aws directory). This fix aligns the help text with the actual WORKSPACE_AGENT_DIR_RELATIVE constant defined in directories.rs.
…urity (aws#2846) - Change default_allow_read_only() from true to false for secure by default behavior - Default behavior: all bash commands require user confirmation (secure by default) - Opt-in behavior: when autoAllowReadonly=true, read-only commands are auto-approved - Use autoAllowReadonly casing to match use_aws tool pattern - Update documentation to reflect new default value and consistent naming - Add comprehensive tests covering all scenarios - Maintains backward compatibility through configuration - Follows same pattern as use_aws autoAllowReadonly setting 🤖 Assisted by Amazon Q Developer Co-authored-by: Matt Lee <mrlee@amazon.com>
- Add Edit subcommand to AgentSubcommands enum - Implement edit functionality that opens existing agent files in editor - Use Agent::get_agent_by_name to locate and load existing agents - Include post-edit validation to ensure JSON remains valid - Add comprehensive tests for the new edit subcommand - Support both --name and -n flags for agent name specification 🤖 Assisted by Amazon Q Developer Co-authored-by: Matt Lee <mr-lee@users.noreply.github.com>
* Change autocomplete shortcut from ctrl-f to ctrl-g The reason is ctrl-f is the standard shortcut in UNIX for moving cursor forward by 1 character. You can find it being supported everywhere... in your browser, your terminal, etc. * make the autocompletion key configurable
* fix incorrect scope for mcp oauth * reverts custom tool config enum change * fixes display task overriding sign in notice * updates schema
aws#2975) * fix: consolidate tool permission logic for consistent display and execution * fix: centralize tool permission checking logic --------- Co-authored-by: Niraj Chowdhary <chownira@amazon.com>
* feat: expand support for /prompts command * fix: prompts spec * fix: add /prompts delete cmd * fix(prompts): improve validation and user input handling * fix(prompts): manage prompt using structs
Co-authored-by: Kenneth S. <kennvene@amazon.com>
Add experimental feature to show context window usage as a percentage in the chat prompt (e.g., "[rust-agent] 6% >"). The percentage is color-coded: green (<50%), yellow (50-89%), red (90-100%). The feature is disabled by default and can be enabled via /experiment.
* [fix] Fixes issues with Tool Input parsing. * Ocassionally the model will generate a tool use which parameters are not a valid json. When this happens it corrupts the conversation history. * Here we first avoid storing the tool use and add the propert validation logic to the conversation history. * adds validation logic to safety * [fix] Update to use a new RecvErrorKind instead of custom error handling. * [fix] Gives visual hint to the user, that request is being retried. --------- Co-authored-by: Kenneth S. <kennvene@amazon.com>
* (in progress) Implement checkpointing using git CLI commands * feat: Add new checkpointing functionality using git CLI Updates: - Only works if the user has git installed - Supports auto initialization if the user is in a git repo, manual if not - UI ported over from dedicated file tools implementation * feat: Add user message for turn-level checkpoints, clean command Updates: - The clean subcommand will delete the shadow repo - The description for turn-level checkpoints is a truncated version of the user's last message * fix: Fix shadow repo deletion logic Updates: - Running the clean subcommand now properly deletes the entire shadow repo for both automatic and manual modes * chore: Run formatter and fix clippy warnings * feat: Add checkpoint diff Updates: - Users can now view diffs between checkpoints - Fixed tool-level checkpoint display handling * fix: Fix last messsage handling for checkpoints Updates: - Checkpoints now (hopefully) correctly display the correct turn-specific user message - Added slash command auto completion * fix: Fix commit message handling again * chore: Run formatter * Removed old comment * define a global capture dirctory * revise the capture path * fix cpature clean bug * add a clean all flag * add auto drop method for capture feature * support file details when expand * add the file summary when list and expand * revise structure and print no diff msg * delete all flag, add summry when fs read * refactor code * revise ui * add capture into experiement * clippy * rename to checkpoint * reverse false renaming * recover history * disable tangent mode in checkpoint * fix cr * nit: keep checkpoint name * allow both tangent & checkpoint enabled * ci --------- Co-authored-by: kiran-garre <kiranbug@amazon.com>
Co-authored-by: Kenneth S. <kennvene@amazon.com>
Add version note indicating that agent-specific knowledge bases are available in development but not in current releases (v1.14.1 and earlier). Current releases use global storage at ~/.aws/amazonq/knowledge_bases/.
When users run 'q settings chat.defaultAgent ""' to reset their default agent (as documented), the empty string was being treated as a valid agent name, causing an error message on every chat session start. This change treats empty strings as 'no default set', allowing users to cleanly reset to the built-in default agent without error messages. Fixes the misleading behavior described in the AWS documentation where setting an empty string should reset to built-in default silently.
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR successfully addresses the main issue described in the title - fixing the empty string handling for the chat.defaultAgent setting. The core fix in crates/chat-cli/src/cli/agent/mod.rs correctly treats empty strings as "no default set" by adding a simple but effective check !user_set_default.is_empty() before attempting agent lookup.
Key Findings
✅ Main Fix: The empty string handling for chat.defaultAgent is properly implemented and maintains backward compatibility.
✅ Code Quality: The PR includes comprehensive test coverage for the new functionality and maintains good code organization.
download_feed_json() function downloads and writes remote content without proper validation, which could be a security risk.
✅ Additional Improvements: Several quality-of-life improvements including better error handling, UI enhancements, and defensive programming practices.
Recommendations
- Address the security vulnerability in the build script by adding content validation and size limits
- Consider the scope: This is a large PR (93 files changed) that includes many features beyond the stated fix - consider breaking such changes into smaller, focused PRs in the future
The core functionality works as intended and the fix is solid. The security issue in the build script should be addressed before merging.
| style::Print("\n"), | ||
| style::SetForegroundColor(Color::Reset) | ||
| ); | ||
| } |
There was a problem hiding this comment.
The fix correctly addresses the issue described in the PR. The change treats empty strings as "no default set" by adding the condition !user_set_default.is_empty() before attempting to find the agent. This prevents the error message when users run q settings chat.defaultAgent "" to reset their default agent.
The logic is sound:
- If the setting exists but is empty, it skips the agent lookup entirely
- If the setting exists and is non-empty, it performs the lookup as before
- If the lookup fails for a non-empty setting, it still shows the appropriate error message
This maintains backward compatibility while fixing the misleading error behavior.
| // Reset pending tool state to prevent orphaned tool approval prompts | ||
| session.tool_uses.clear(); | ||
| session.pending_tool_index = None; | ||
| session.tool_turn_start_time = None; |
There was a problem hiding this comment.
Good improvement to reset pending tool state when clearing the conversation. This prevents orphaned tool approval prompts that could confuse users after a clear operation. The additional cleanup of tool_uses, pending_tool_index, and tool_turn_start_time ensures a clean slate.
| }, | ||
| } | ||
| } else { | ||
| debug!("Custom endpoint detected, skipping profile ARN"); |
There was a problem hiding this comment.
Good defensive programming to check for custom endpoints before attempting to get the auth profile. This prevents unnecessary error logging when using custom endpoints that don't require profile ARNs. The helper method is_custom_endpoint provides a clean abstraction for this check.
| #[error("{}", SdkErrorDisplay(.0))] | ||
| SdkRegisterClient(Box<SdkError<RegisterClientError>>), | ||
| #[error(transparent)] | ||
| #[error("{}", SdkErrorDisplay(.0))] |
There was a problem hiding this comment.
The introduction of SdkErrorDisplay wrapper for better error formatting is a good improvement. This provides more user-friendly error messages for SDK-related authentication errors instead of the raw debug output.
| #[serde(default)] | ||
| pub use_legacy_mcp_json: bool, | ||
| /// The model ID to use for this agent. If not specified, uses the default model. | ||
| #[serde(default)] |
There was a problem hiding this comment.
The addition of the model field to the Agent struct is well-implemented with proper serialization/deserialization support and comprehensive test coverage. The field correctly defaults to None and is properly handled in the Default implementation.
| fn default_permission_label(&self, tool_name: &str) -> String { | ||
| let label = match tool_name { | ||
| "fs_read" => "trusted".dark_green().bold(), | ||
| "fs_read" => "trust working directory".dark_grey(), |
There was a problem hiding this comment.
Good improvement to the default permission labels. Changing fs_read from "trusted" to "trust working directory" provides more specific information about the scope of trust, and changing execute_bash/execute_cmd from "trust read-only commands" to "not trusted" is more accurate and secure by default.
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Problem
The AWS documentation states that users can reset their default agent by running:
q settings chat.defaultAgent ""However, this currently causes an error message on every chat session start:
Root Cause
The issue occurs because:
""stores it as a valid string value in the databaseget_string()returnsSome("")for empty strings, notNone""(empty string)Solution
This PR modifies the agent selection logic to treat empty strings as "no default set", allowing users to cleanly reset to the built-in default agent without error messages.
Changes
crates/chat-cli/src/cli/agent/mod.rsto check!user_set_default.is_empty()before attempting to find the agentTesting
The fix ensures that:
q settings chat.defaultAgent ""works as documented (no error messages)q settings chat.defaultAgent "invalid-agent"still shows error messagesFixes the misleading behavior described in the AWS documentation where setting an empty string should reset to built-in default silently.
NOTE: The documentation will also be changed to (at least temporarily) stop mentioning the problematic command, and will also recommend the working command "q setting chat.defaultAgent --delete".
Ready to be submitted as PR to upstream aws/amazon-q-developer-cli.