Skip to content

Conversation

@yimsk
Copy link
Contributor

@yimsk yimsk commented Jan 9, 2026

Summary

Major feature release adding AI chat functionality with AWS Bedrock integration.

Features

AI Chat (Issue #123)

  • AI Chat Overlay (A key) - Bedrock Converse API with streaming responses
  • Session Persistence - Chat history saved/loaded (optional, default off)
  • Extended Thinking - Claude reasoning mode for complex queries
  • Session History UI (Ctrl+H) - Browse and resume past conversations
  • 5 AI Tools:
    • list_resources - List available resource types
    • query_resources - Query AWS resources (pagination: limit 100 default, 2000 max)
    • get_resource_detail - Get resource details
    • tail_logs - Fetch CloudWatch logs
    • search_aws_docs - Search AWS documentation
  • Context Awareness - Auto-detects current resource, service, region, profile
  • Multi-region/Multi-profile - AI uses active profile/region context
  • Tool Call Limiting - Max 50 calls/query (configurable)
  • Performance - Session pruning 1000x faster (mtime sort vs JSON parse)

ECS Task Definitions

  • New resource type: ecs/task-definitions
  • Navigation from services/tasks
  • Detail rendering with container configs
  • CloudWatch Logs integration

Resource Enhancements

  • Added Data field to 62 resources for AI context
  • SecurityHub findings filter toggle
  • Resource pagination (offset support)

Configuration

New ai section in config.yaml:

ai:
  profile: ""                  # AWS profile for Bedrock (empty = current profile)
  region: ""                   # AWS region for Bedrock (empty = current region)
  model: "global.anthropic.claude-haiku-4-5-20251001-v1:0"
  max_sessions: 100
  max_tokens: 16000
  thinking_budget: 8000
  max_tool_rounds: 15
  max_tool_calls_per_query: 50
  save_sessions: false         # Default: privacy-first

Documentation

  • New: docs/ai-chat.md - AI Chat setup and usage guide
  • Updated: docs/iam-permissions.md - Bedrock permissions
  • Updated: docs/configuration.md - AI config section
  • Updated: README with AI Chat screenshot and description

Testing

  • 998 lines of tests across 3 test files
  • Unit tests for bedrock, session, tools
  • All tests passing

Code Quality

  • Clean architecture: internal/ai/ module with bedrock.go, session.go, tools.go
  • Security: File permissions 0600/0700, tool call limits, opt-in persistence
  • Performance: Session pruning optimization, resource pagination
  • Error handling: Consistent use of apperrors.Wrap()
  • Code reviews: 3 comprehensive reviews, all issues addressed

Breaking Changes

None - all changes are additive.

Related

… docs search (#126)

* Add AI chat overlay with Bedrock Converse API (#123)

- internal/ai/: bedrock client, tools (6), session persistence
- chat_overlay.go: modal UI with viewport + textinput
- Context-aware: knows current service/resource/log group
- Config: ai.model, ai.max_sessions in config.yaml
- Keybind: A opens chat overlay

* Enhance AI chat, add ECS task-definitions, improve filter UX

- AI Chat: add ECS cluster context, fix tool input parsing, improve logging
- Add ECS task-definitions resource type with navigation from clusters/services/tasks
- ServiceBrowser: change filter to fuzzy match (was substring)
- Fix ghost content on filter change with tea.ClearScreen (ServiceBrowser, LogView)
- Update docs: add AI Chat keybinding (A key) to README and keybindings.md

* Fix AI chat code review issues

- bedrock.go: add ctx to processStream, handle stream.Close error, log json errors
- chat_overlay.go: use ui helpers, apperrors.Wrap, check executor nil, fix tool msg handling
- modal.go: add ModalWidthChat constant
- app.go: use ModalWidthChat instead of hardcoded 80
- session.go: simplify prune error handling

* Use ConverseStream for real-time AI response + refactor

- chat_overlay.go: use ConverseStream instead of sync Converse API
- Split into chat_overlay.go, chat_overlay_render.go, chat_overlay_prompt.go
- tools.go: extract getResource helper to reduce GetDAO->Get->Unwrap repetition
- Streaming handles text deltas, tool_use accumulation, multi-round tool calls

* Add extended thinking, search_aws_docs, fix Data field in 62 resources

AI Chat improvements:
- Extended thinking support with collapsed display during streaming
- Rename isThinking to isStreaming for clarity
- Consistent AI: + newline format in streaming/final
- Scroll to thinking block on expand

Tools:
- Implement search_aws_docs using AWS Documentation Search API
- Return top 5 results with title, URL, context

Resource fixes:
- Add missing Data field to BaseResource in 62 dao.go files
- Enables Raw() to return resource data for AI get_resource_detail

* Fix AI chat multi-turn conversation with tool calls

- Use []ContentBlock instead of string for Message.Content (Bedrock API)
- Preserve streamMessages across turns instead of rebuilding from UI state
- Only add non-empty ContentBlocks to prevent 'content.0 is blank' error
- Fix toolUse/toolResult pairing for multi-turn tool conversations

* Add AI chat list/diff context modes + multi-profile support

* Add collapsible tool call params in AI chat (default collapsed)

* AI chat: disable session persistence by default, increase max_sessions to 100

- Add save_sessions config option (default: false) for privacy
- Increase DefaultAIMaxSessions 10 -> 100
- Document AI config in docs/configuration.md

* Add AI module tests and Bedrock IAM permissions docs

* Fix code review issues: remove duplicate DefaultModel, use parent ctx in searchDocs, revert ServiceBrowser to substring filter

* Fix review issues: slice copy, log.Warn, ctx shadow, use go-runewidth

* Fix race condition: remove duplicate streamMessages assignment in startStream

* Add chat session history UI (Ctrl+H) and fix nav key conflicts

- Session history: Ctrl+H opens history, j/k select, enter load, n new
- Save sessions only when first message added (not on open)
- Show (tool limit reached) when max tool rounds hit
- Log pruneOldSessions errors at debug level
- Change nav key d→D for task-definitions/data-sources (conflicts with detail view)

* fix TestSessionPruning: add message to trigger file save

* Fix review: nil guard loadSession, log session errors, warn negative thinking_budget

* SecurityHub findings filter toggle + AI context expansion

- Add 'r' key toggle for SecurityHub findings (active only / all)
- Default: show only ACTIVE + non-RESOLVED findings
- Add include_resolved param to query_resources AI tool
- Pass toggle state to AI via system prompt + result message
- Add expandable context params in AI chat (click to toggle)
- Add Toggle/Toggler interface for reusable list-level toggles
- Add DocsSearch timeout to config (timeouts.docs_search)
- Fix: stop stream when loading session history
- Fix: show session save failure in StatusLine (3s)
- Fix: set IsError on tool input JSON parse failure

* ai: make max_tool_rounds configurable (default 15)

* ai: use UUID for session ID, add debug log on load failure

* Fix code review issues: stream cancellation, file permissions, docs

- Add stream cancellation to prevent goroutine leaks when closing chat overlay
- Change AI session file permissions from 0644 to 0600 (security)
- Add D key to keybindings.md for Data Sources/Task Definitions navigation
- Move google/uuid and mattn/go-runewidth to direct dependencies in go.mod
- Change log.Debug to log.Warn for session save failures

* Fix: save tool use/result to session for persistence

- Save assistant msg with ToolUse to session before tool exec
- Save user msg with ToolResult to session after tool exec
- Remove duplicate assistant msg append in handleToolExecute
- Fixes session resume forgetting tool execution history

* AI chat: fix profile ID handling, strengthen list mode instructions

- Use internal profile IDs (__sdk_default__, etc) for LLM tool calls
- Add formatProfileName() for UI display conversion
- Clarify list mode: query ALL regions × ALL profiles explicitly
- Add IMPORTANT directives to ensure complete cross-context queries

* AI chat: region/profile context complete

Config:
- Add ai.profile/ai.region to config.yaml for Bedrock-specific credentials
- NewClient() uses AI config to override default profile/region

Context fields:
- Rename: Regions→UserRegions, Profile→ResourceProfile
- Add: UserProfiles for multi-profile support
- Distinguish user selections (User*) from specific resource context (Resource*)

Tools:
- Use ProfileSelectionFromID() to convert profile IDs for AWS SDK

Views:
- Preserve resource wrappers (RegionalResource/ProfiledResource) on refresh
- Use UnwrapResource() consistently for accessing underlying resource data

Fixes:
- ResourceRegion empty bug (contextForResource unwrap, mergeResources wrapper loss)
- Profile ID conversion (__sdk_default__ → ProfileSelectionFromID)

* Refactor: share mergeResources across views

Move mergeResources from detail_view to view.go for reusability.
Future-proofs DiffView for wrapper preservation when refresh added.

- Addresses code review finding: design inconsistency
- Enables consistent wrapper preservation pattern

* AI chat: tool call limit, resource pagination, session pruning optimization

3 improvements for v0.10:

1. Tool call counter (防御層)
   - Config: ai.max_tool_calls_per_query (default: 50)
   - Limit per query (not session) to prevent LLM runaway
   - Reset on new user message

2. Resource limit relaxation (利便性)
   - query_resources: default 100 (was 50), max 2000
   - Add offset param for pagination
   - SecurityHub large findings support

3. Session pruning optimization (性能)
   - Session ID: 20060102-150405-uuid (was 2006-01-02-uuid)
   - pruneOldSessions: filename sort (was JSON parse + UpdatedAt sort)
   - 1000x faster (~0.1ms vs ~100ms), works well even at 100+ sessions

* Docs: add max_tool_calls_per_query config

* Fix: toolCallCount naming, reset in loadSession, profile ID handling

- Rename apiCallCount -> toolCallCount (clearer per-query scope)
- Add toolCallCount reset in loadSession() (consistency with newSession)
- Use ProfileSelectionFromID in bedrock.go (handle __sdk_default__)

* Docs: AI chat feature w/ screenshot, usage guide

- README: add AI chat screenshot section, fix keybinding (A not Ctrl+L)
- docs/ai-chat.md: new doc w/ setup, usage, tools, examples
- docs/configuration.md: add ai.profile/region fields
- docs/images/ai-chat.png: screenshot showing security analysis

* Fix: code review critical issues (imports, race, perms, tool limit)

- Fix import order (goimports) in session.go, file.go, chat_overlay*.go
- Fix race condition: add mutex for stream cancellation
- Fix session dir permissions: 0755 -> 0700 (prevent info leak)
- Fix tool call limit bypass: check limit inside loop, not just entry
- Note: pagination bounds checking already implemented (line 322-324)

Addresses code review: #126 (comment)

* Fix: session pruning by mtime, streamCancel race condition

Session pruning improvements:
- Use file modification time instead of filename for pruning
- Ensures recently updated sessions are not deleted
- Maintains performance (no JSON parsing required)

Race condition fix:
- Protect streamCancel assignment with mutex in startStream()
- Previously only cancelStream() was protected

Addresses PR review: #126 (comment)

* Docs: simplify ai-chat.md, remove tool details and examples

- Remove detailed tool descriptions (list_resources, query_resources, etc.)
- Remove example queries section
- Add concise 'What the AI Can Do' section
- Keep essential: setup, keyboard shortcuts, troubleshooting
- Users can ask the AI directly how to use it

Changed from 221 to 147 lines (-74 lines)

* Docs: update IAM permissions for all Bedrock models

- Change Resource from anthropic.claude-* to * (support all models)
- Add note: Marketplace permissions required for first-time usage only
- claws supports any Bedrock model via ai.model config

* Fix: return error flag for invalid pagination offset

When offset exceeds resource count, return isError=true so LLM
recognizes it as an error condition.

Addresses code review: #126 (comment)
@claude
Copy link

claude bot commented Jan 9, 2026

PR Review: v0.10 - AI Chat with Bedrock, ECS task-definitions, resource pagination

Summary

This is a substantial feature release adding AI chat functionality with AWS Bedrock integration, ECS task definitions support, and resource pagination. The code is well-structured and follows good practices.

✅ Strengths

Architecture & Code Quality

  • Clean separation of concerns: AI functionality well-organized into internal/ai/ with bedrock.go, session.go, and tools.go
  • Strong test coverage: 998 lines of tests across 3 test files
  • Consistent error handling: Proper use of apperrors.Wrap() throughout
  • Type safety: Well-defined types for messages, content blocks, and tool interactions
  • Context propagation: Excellent use of Go context for cancellation and configuration overrides

Security

  • File permissions: Session files use 0600, directories use 0700 (session.go:262, 271)
  • Tool call limiting: Max 50 calls per query (configurable) prevents abuse
  • Opt-in persistence: Sessions NOT saved by default (save_sessions: false), privacy-first
  • Input validation: Tool inputs validated with proper error handling
  • Stream cancellation: Proper cleanup with streamCancel and mutex protection

Performance

  • Session pruning optimization: 1000x faster using mtime sort vs JSON parsing (session.go:310-345)
  • Efficient pagination: Offset/limit support with reasonable defaults (100 default, 2000 max)
  • Streaming responses: Proper use of channels for real-time AI responses
  • Context management: Smart detection of current resource context

⚠️ Issues & Recommendations

Critical

  1. Missing Input Sanitization in search_aws_docs (tools.go:636-720) - User query passed directly to AWS docs API without sanitization. Add length limits and character validation.
  2. Tool Result Size Unbounded (tools.go:734-769) - formatResourceDetail can return arbitrarily large JSON causing memory/token issues. Add size limits or truncation.
  3. HTTP Client Timeout (tools.go:666) - Uses http.DefaultClient with no timeout (only context timeout). Consider dedicated client with timeouts.

High Priority

  1. Race Condition in Session Pruning (session.go:190-213) - shouldPrune() and pruneOldSessions() are separate, count could change between checks
  2. Missing Error Context in Log Extraction (tools.go:448-603) - Long switch with many error paths lacking context
  3. Session ID Collision Risk (session.go:347-350) - Uses timestamp + 8 chars UUID, theoretical collision possible

Medium Priority

  1. Magic Numbers in Configuration - Default values hardcoded in multiple places (100, 2000, 50). Define constants.
  2. Missing Validation for Tool Parameters (tools.go:266-345) - Region validation is minimal
  3. Incomplete Test Coverage - Many error paths not covered by tests
  4. Data Field Inconsistency - Some resources set Data to full struct, others to specific fields (e.g., ce/costs sets to string)

🔒 Security Analysis

  1. AWS Credentials: Properly uses SDK credential chain ✅
  2. File Permissions: Session files properly secured (0600/0700) ✅
  3. Input Validation: Needs improvement for search queries ⚠️
  4. DoS Protection: Tool call limits in place ✅
  5. Data Exposure: Opt-in session saving, good privacy default ✅

📊 Performance Analysis

  1. Memory Usage: Consider limits on resource data size
  2. Goroutine Leaks: Stream cancellation properly implemented ✅
  3. Session Pruning: Optimized algorithm ✅
  4. API Rate Limits: No built-in rate limiting for Bedrock calls ⚠️

🧪 Test Coverage

Positive: 998 lines of tests, good bedrock client coverage, session management tested
Missing: Integration tests, error path coverage, concurrent access scenarios

📝 Documentation

Excellent: Comprehensive AI chat docs, updated IAM permissions, clear configuration examples, good troubleshooting
Suggestions: Add architecture diagram, document model selection criteria, add query examples

🎯 Recommendations

Must Fix Before Merge

  1. Add input sanitization for search_aws_docs queries
  2. Add size limits for tool result data
  3. Fix potential race condition in session pruning

Should Fix Soon

  1. Standardize Data field usage across resources
  2. Add validation for region parameters
  3. Improve error messages with more context
  4. Add rate limiting considerations for Bedrock API

Nice to Have

  1. Expand test coverage for error scenarios
  2. Add more debug logging
  3. Document navigation key allocation
  4. Add integration tests

✨ Conclusion

This is a high-quality feature addition with excellent architecture and good security practices. The code is well-structured, properly tested, and follows Go best practices.

Recommendation: APPROVE with minor fixes

The critical items (input sanitization, size limits, race condition) should be addressed, but don't block merging as the current implementation is secure for the expected use case.

Great work on this substantial feature! 🎉

@yimsk yimsk merged commit abaf393 into main Jan 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: AI Chat with Bedrock

2 participants