fix: treat empty string as unset for chat.defaultAgent setting

[mibeco]

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:

Error: user defined default  not found. Falling back to in-memory default

Root Cause

The issue occurs because:

1. Setting an empty string "" stores it as a valid string value in the database
2. get_string() returns Some("") for empty strings, not None
3. The agent selection logic tries to find an agent named "" (empty string)
4. When it fails to find this agent, it shows the error message

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

• Modified crates/chat-cli/src/cli/agent/mod.rs to check !user_set_default.is_empty() before attempting to find the agent
• Empty strings now silently fall through to the built-in default agent
• Non-empty invalid agent names still show appropriate error messages

Testing

The fix ensures that:

• q settings chat.defaultAgent "" works as documented (no error messages)
• q settings chat.defaultAgent "invalid-agent" still shows error messages
• Normal agent selection continues to work as expected

Fixes the misleading behavior described in the AWS documentation where setting an empty string should reset to built-in default silently.

[amazon-q-developer]

⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion.

Using Amazon Q Developer for GitHub

Amazon Q Developer¹ 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

Command	Description
/q <message>	Chat with the agent to ask questions or request revisions
/q review	Requests an Amazon Q powered code review
/q help	Displays usage information

Features

Agentic Chat
Enables interactive conversation with Amazon Q to ask questions about the pull request or request specific revisions. Use /q <message> in comment threads or the review body to engage with the agent directly.

Code Review
Analyzes pull requests for code quality, potential issues, and security concerns. Provides feedback and suggested fixes. Automatically triggered on new or reopened PRs (can be disabled for AWS registered installations), or manually with /q review slash command in a comment.

Customization

You can create project-specific rules for Amazon Q Developer to follow:

1. Create a .amazonq/rules folder in your project root.
2. Add Markdown files in this folder to define rules (e.g., cdk-rules.md).
3. Write detailed prompts in these files, such as coding standards or best practices.
4. Amazon Q Developer will automatically use these rules when generating code or providing assistance.

Example rule:

All Amazon S3 buckets must have encryption enabled, enforce SSL, and block public access.
All Amazon DynamoDB Streams tables must have encryption enabled.
All Amazon SNS topics must have encryption enabled and enforce SSL.
All Amazon SNS queues must enforce SSL.

Feedback

To 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

1. Amazon Q Developer uses generative AI. You may need to verify generated code before using it in your environment. See the AWS Responsible AI Policy. ↩

[amazon-q-developer]

Summary

This PR successfully addresses the core issue described in the title - fixing empty string handling for the chat.defaultAgent setting. The main fix in crates/chat-cli/src/cli/agent/mod.rs correctly treats empty strings as "no default set" rather than attempting to find an agent with an empty name.

Key Findings

✅ Positive Changes

• Core Fix: The empty string check (!user_set_default.is_empty()) properly resolves the documented behavior issue
• Security Improvements: Custom endpoint detection prevents credential leakage to non-AWS endpoints
• Error Handling: Better SDK error formatting with SdkErrorDisplay wrapper

⚠️ Security Concerns

• Command Injection Risks: Both download_feed_json() in build.rs and run_git() in checkpoint.rs execute external commands with insufficient input validation
• Network Security: The build script downloads content from GitHub without integrity verification

📋 Recommendations

1. High Priority: Address command injection vulnerabilities in build.rs and checkpoint.rs
2. Medium Priority: Consider more conservative version bumping (patch vs minor for bug fixes)
3. Low Priority: Add test coverage for the empty string handling behavior

Scope Concern

While the PR title suggests a focused bug fix, this PR includes substantial new functionality (93 files changed, 8695 additions). Consider splitting large feature additions from critical bug fixes in future PRs for easier review and safer deployment.

The core fix is solid and addresses the documented issue effectively. The security vulnerabilities should be addressed before merging.

[amazon-q-developer]

✅ Excellent Fix: This change correctly addresses the issue described in the PR. The addition of !user_set_default.is_empty() check ensures that empty strings are treated as "no default set" rather than attempting to find an agent with an empty name.

This allows users to cleanly reset their default agent using q settings chat.defaultAgent "" as documented, without triggering the error message. The logic properly falls through to the built-in default agent when an empty string is provided, which is the expected behavior.

[amazon-q-developer]

Suggestion: Consider adding a test case to verify the empty string handling behavior. This would help ensure the fix continues to work correctly and documents the expected behavior.

[amazon-q-developer]

The version bump from 1.15.0 to 1.16.3 seems significant for what appears to be a bug fix. Consider using semantic versioning more strictly - this type of bug fix would typically warrant a patch version increment (e.g., 1.15.1) rather than a minor version jump to 1.16.3.

[amazon-q-developer]

🛑 Security Vulnerability: The download_feed_json() function has several security concerns:

1. Command Injection Risk: The function executes curl with arguments that could potentially be manipulated if the URL or other parameters were user-controlled.

2. Network Security: Downloads content from GitHub API without certificate validation or integrity checks.

3. File System Risk: Overwrites src/cli/feed.json without proper validation of the downloaded content.

Recommendations:

• Validate the downloaded JSON content before writing to disk
• Add checksum verification for the downloaded file
• Consider using a Rust HTTP client instead of shelling out to curl
• Add timeout and size limits for the download

[amazon-q-developer]

Code Quality Issue: Using panic! in build scripts can cause confusing build failures. Consider using cargo:warning= directives to emit warnings instead of panicking, or provide more graceful fallback behavior when curl is not available or network requests fail.

This would make the build more resilient in environments where network access is restricted or curl is not installed.

[amazon-q-developer]

🛑 Command Injection Vulnerability: The run_git function constructs git commands with user-controlled input without proper sanitization. The args parameter could potentially contain malicious command arguments.

Recommendation: Validate and sanitize all arguments passed to git commands, or use a git library instead of shelling out to the git binary.

Suggested change

	}
	// Tag
	let output = run_git(Path::new(shadow_path), false, &["tag", tag])?;
	fn run_git(dir: &Path, with_work_tree: bool, args: &[&str]) -> Result<Output> {
	// Validate git arguments to prevent command injection
	for arg in args {
	if arg.contains(';') || arg.contains('|') || arg.contains('&') {
	bail!("Invalid git argument: {}", arg);
	}
	}
	let mut cmd = Command::new("git");
	cmd.arg(format!("--git-dir={}", dir.display()));
	if with_work_tree {
	cmd.arg("--work-tree=.");
	}
	cmd.args(args);
	let output = cmd.output()?;
	if !output.status.success() && !output.stderr.is_empty() {
	bail!(String::from_utf8_lossy(&output.stderr).to_string());
	}
	Ok(output)
	}

[amazon-q-developer]

Good Practice: The addition of SdkErrorDisplay wrapper for better error formatting is a good improvement. This provides more user-friendly error messages while maintaining the underlying error information for debugging.

[amazon-q-developer]

Good Security Practice: The addition of custom endpoint detection (is_custom_endpoint) and conditional profile handling is a good security improvement. This prevents attempting to use AWS profiles when connecting to non-AWS endpoints, which could potentially leak credentials or cause authentication errors.