Skip to content

AI Chat: Bedrock integration with streaming, session persistence, AWS docs search#126

Merged
yimsk merged 33 commits intodevelopfrom
feature/123-ai-chat
Jan 9, 2026
Merged

AI Chat: Bedrock integration with streaming, session persistence, AWS docs search#126
yimsk merged 33 commits intodevelopfrom
feature/123-ai-chat

Conversation

@yimsk
Copy link
Copy Markdown
Contributor

@yimsk yimsk commented Jan 9, 2026

Summary

  • AI chat overlay (Ctrl+L) with Bedrock Converse API streaming
  • Session persistence with tool use/result tracking
  • Extended thinking mode for complex queries
  • AWS docs search tool (search_aws_docs)
  • Session history UI (Ctrl+H)
  • Tool call limit (max 50/query, configurable)
  • Resource pagination (100 default, 2000 max)
  • Session pruning optimization (1000x faster)

Configuration

New AI section in config.yaml:

ai:
  profile: ""        # AWS profile for Bedrock (empty = current profile)
  region: ""         # AWS region (empty = current region)
  max_tool_calls_per_query: 50
  max_tool_rounds: 15
  session_persistence: false
  max_sessions: 100

Test Plan

  • Unit tests pass (bedrock_test.go, session_test.go, tools_test.go)
  • Code review complete (no blocking issues)
  • Manual testing: chat overlay lifecycle
  • Manual testing: session save/load
  • Manual testing: tool execution (list/getresources)
  • Manual testing: multi-turn conversations
  • Manual testing: stream cancellation
  • Manual testing: region/profile context

Related

Closes #123

yimsk added 27 commits January 7, 2026 17:19
- 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
- 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
- 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
- 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
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
- 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
…s to 100

- Add save_sessions config option (default: false) for privacy
- Increase DefaultAIMaxSessions 10 -> 100
- Document AI config in docs/configuration.md
… in searchDocs, revert ServiceBrowser to substring filter
- 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)
- 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
- 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
- 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
- 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
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)
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
…zation

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
- Rename apiCallCount -> toolCallCount (clearer per-query scope)
- Add toolCallCount reset in loadSession() (consistency with newSession)
- Use ProfileSelectionFromID in bedrock.go (handle __sdk_default__)
@claude

This comment was marked as outdated.

- 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
@claude

This comment was marked as resolved.

- 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)
@claude

This comment was marked as resolved.

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)
@claude

This comment was marked as resolved.

@claude

This comment was marked as outdated.

- 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)
@yimsk yimsk force-pushed the feature/123-ai-chat branch from f4987e5 to 81f1789 Compare January 9, 2026 12:47
@claude

This comment was marked as resolved.

- 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
@claude

This comment was marked as resolved.

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

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

claude Bot commented Jan 9, 2026

Pull Request Review: AI Chat with Bedrock Integration

This is an impressive and well-executed feature addition! The AI chat overlay with AWS Bedrock integration adds significant value to claws. The implementation demonstrates good architectural design, comprehensive testing, and attention to detail.


✅ Strengths

1. Architecture & Code Quality

  • Clean separation of concerns: The internal/ai/ module is well-structured with distinct responsibilities:
    • bedrock.go: API client and streaming
    • session.go: Session persistence
    • tools.go: Tool execution
  • Consistent error handling: Proper use of apperrors.Wrap() throughout
  • Context propagation: Excellent use of context for cancellation, region/profile overrides, and filters
  • Stream cancellation: Proper cleanup with mutex-protected streamCancel to prevent goroutine leaks (lines 98-238 in chat_overlay.go)

2. Security

  • File permissions: Session files use 0600 and directories use 0700 - excellent for protecting sensitive data
  • Tool call limits: Defense-in-depth with max_tool_calls_per_query (default 50) to prevent LLM runaway
  • Input validation: Proper bounds checking on pagination offset/limit parameters
  • Safe resource access: Profile/region isolation through AWS SDK context

3. Performance

  • Session pruning optimization: Smart use of file modification time instead of JSON parsing - 1000x faster!
  • Resource pagination: Sensible defaults (100) with max limit (2000) to prevent memory issues
  • Concurrent region fetching: Already leverages existing multi-region infrastructure
  • Streaming response: Real-time UI updates via ConverseStream API

4. User Experience

  • Session persistence: Optional (default off for privacy) with history UI (Ctrl+H)
  • Extended thinking: Collapsible reasoning blocks for complex queries
  • Context awareness: Automatically detects current resource, service, region, profile
  • Tool result display: Collapsible tool calls to reduce noise
  • Error feedback: Clear error messages with actionable guidance

5. Testing

  • Comprehensive coverage: 998 lines of tests across 3 test files
    • bedrock_test.go: 287 lines
    • session_test.go: 385 lines
    • tools_test.go: 326 lines
  • Unit tests: Well-isolated tests for core functionality
  • Edge cases: Tests cover error conditions, empty states, limits

6. Documentation

  • New doc: docs/ai-chat.md with setup and usage guide
  • IAM permissions: Clear documentation of required Bedrock permissions
  • Configuration: Well-documented AI config section with sensible defaults
  • README: Updated with screenshot and feature description

🔍 Code Quality Observations

Excellent Practices

  1. Type safety: Strong typing with custom types (Role, StopReason, ContextMode)
  2. Helper functions: Clean abstractions like NewUserMessage(), formatResourceDetail()
  3. Stream processing: Proper state machine for handling streaming events
  4. Resource unwrapping: Consistent use of dao.UnwrapResource() for wrapper handling
  5. Timeout handling: Context-based cancellation in AWS docs search

Minor Observations

  1. UUID dependency: Added as direct dependency (good - not indirect)
  2. Magic numbers: Some constants could be named (e.g., 10 buffer size on line 168)
  3. Error logging: Consistent use of log.Debug/Warn - good logging hygiene

🛡️ Security Considerations

✅ Well Handled

  • Session data protected with proper file permissions
  • Tool execution sandboxed to read-only AWS API calls
  • No command injection risks (uses AWS SDK, not shell commands)
  • Profile/region isolation prevents cross-account leaks
  • Tool call limits prevent abuse

💡 Recommendations (Future)

  1. Rate limiting: Consider per-session rate limits for API calls
  2. Audit logging: Consider logging AI queries for compliance (opt-in)
  3. Token budget: The thinkingBudget is configurable - good for cost control

🐛 Potential Issues

No Critical Issues Found! ✨

Minor Considerations

  1. tools.go:322-324: Offset validation correctly returns isError=true - good catch from review
  2. Pagination bounds: Properly handled with clear error messages
  3. Race condition: Fixed with mutex on streamCancel assignment
  4. Session directory: Created with 0700 permissions - excellent

🧪 Test Coverage

The test coverage is excellent:

  1. bedrock_test.go: Tests client initialization, message conversion, tool configuration
  2. session_test.go: Tests session creation, persistence, pruning, history
  3. tools_test.go: Tests all 5 tools with various scenarios

Strengths:

  • Tests use table-driven approach
  • Edge cases covered (empty inputs, invalid offsets, missing resources)
  • Session pruning tested with file count verification

📋 Best Practices Adherence

Follows Project Conventions ✅

  1. DAO pattern: tools.go properly uses registry.GetDAO() and context filters
  2. Error wrapping: Consistent use of apperrors.Wrap()
  3. Config management: Uses config.File() for all configuration
  4. Import organization: Fixed with goimports in final commits
  5. Generated files: Properly added task-definitions/constants.go

Architecture Alignment ✅

  1. Registry usage: Proper integration with resource registry
  2. Context filtering: Follows established patterns for ECS cluster context
  3. Multi-region support: Works seamlessly with existing infrastructure
  4. Modal system: Follows existing modal patterns (width constants, lifecycle)

🚀 Performance Considerations

✅ Optimized

  1. Session pruning: O(n log n) file sorting vs O(n) JSON parsing - huge win
  2. Resource limits: Prevents unbounded API calls with sensible defaults
  3. Stream buffering: 10-event buffer on channel to reduce blocking
  4. Lazy loading: Sessions loaded on-demand, not upfront

💡 Future Optimizations

  1. Caching: Consider caching AWS docs search results (future enhancement)
  2. Tool result truncation: Very large resources might overwhelm context (already has limits)
  3. Concurrent tool execution: Currently sequential - could parallelize independent tools (future)

📊 Metrics

  • Lines Added: 5,480
  • Lines Deleted: 234
  • Files Changed: 93
  • Test Files: 3 (998 lines of tests)
  • Commits: 32 (incremental improvements based on reviews)
  • Services Enhanced: 62 resources got Data field added for AI access

🎯 Recommendations

Before Merge

✅ All items addressed in the PR already!

Future Enhancements (Out of Scope)

  1. Multi-turn conversation optimization: Implement conversation summarization for long sessions
  2. Tool call caching: Cache identical tool calls within same session
  3. Parallel tool execution: Execute independent tools concurrently
  4. Cost tracking: Optional per-session token/cost tracking
  5. Export conversations: Export chat history as markdown/JSON

🏆 Final Assessment

Recommendation: ✅ APPROVE and MERGE

This PR represents exceptional work:

  • ✅ Clean architecture with excellent separation of concerns
  • ✅ Comprehensive testing (998 lines)
  • ✅ Strong security posture (file permissions, rate limits)
  • ✅ Performance optimizations (1000x faster pruning)
  • ✅ Great UX (session history, collapsible sections, context awareness)
  • ✅ Well documented (new doc, updated README, IAM guide)
  • ✅ Follows all project conventions and patterns
  • ✅ No critical issues or bugs identified

The 32 commits show iterative refinement based on code review feedback, demonstrating excellent engineering discipline. The feature is production-ready and adds significant value to claws.

Excellent work! 🎉


📝 Detailed Comments by File

internal/ai/bedrock.go

  • Line 175-213: Well-structured thinking config with proper beta flag
  • Line 299-416: Stream processing handles all event types correctly
  • Line 230-275: Content block conversion handles all cases (text, tool use, tool result, reasoning)

internal/ai/session.go

  • Line 260-263: Directory permissions 0700 - excellent security
  • Line 265-273: File permissions 0600 - excellent security
  • Line 296-345: Pruning optimization using mtime - brilliant!

internal/ai/tools.go

  • Line 266-345: Resource pagination with proper bounds checking
  • Line 322-324: Offset validation returns isError=true
  • Line 636-720: AWS docs search with proper timeout handling

internal/view/chat_overlay.go

  • Line 98-101: Mutex-protected stream cancel - prevents race conditions ✅
  • Line 231-238: Proper cleanup on cancel
  • Line 274-284: Session persistence with error handling

custom/ecs/task-definitions/

  • Well-implemented new resource type
  • Proper navigation from services/tasks
  • Log group extraction for tail_logs tool

Review completed with focus on code quality, security, performance, testing, and architecture alignment.

@yimsk yimsk merged commit 6168f63 into develop Jan 9, 2026
5 checks passed
@yimsk yimsk deleted the feature/123-ai-chat branch January 9, 2026 13:33
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.

1 participant