fix: remove timeout from squash summarization to support local LLMs

[alvinunreal]

Removed the hard-coded 30-second timeout that was causing context deadline exceeded errors when using local LLM instances with large contexts. The summarization request now uses an unbounded context, allowing it to complete regardless of how long the local LLM takes.

Fixes #80

[alvinunreal]

@claude add also in readme how to isntall from maste latest version

[claude]

Claude finished @alvinunreal's task —— View job

---

Installation instructions added ✓

• Read CLAUDE.md for repo guidelines
• Find and read the README file
• Add instructions for installing from master
• Commit and push changes

Summary

I've added a new section to the README titled "Install from Master (Latest Development Version)" with instructions on how to install TmuxAI from the master branch.

The new section includes:

• Manual build instructions (clone → build → install)
• Quick install using go install github.com/alvinunreal/tmuxai@master
• A note warning that master may be less stable than releases
• Updated Table of Contents with the new section

The changes have been committed and pushed to this PR branch.

Files changed:

• README.md - Added installation instructions for master branch (README.md:107-129)

---

Job Run | Branch: claude/issue-80-20251013-1224

[claude]

Pull Request Review: Remove Timeout from Squash Summarization

Summary

This PR removes the hard-coded 30-second timeout from the squash summarization feature to support local LLM instances that may require more time to process large contexts. The change is minimal and directly addresses issue #80.

---

✅ Code Quality & Best Practices

Strengths:

• Minimal, focused change: The PR follows the single responsibility principle by only addressing the timeout issue
• Clear commenting: The updated comment explains the rationale for removing the timeout
• Consistent with Go idioms: Using context.Background() is appropriate for operations without specific cancellation requirements

Suggestions:

1. Consider making timeout configurable: Instead of removing the timeout entirely, consider making it configurable via the config file:

   // In config
   SquashTimeout int // 0 means no timeout
   
   // In squash.go
   var ctx context.Context
   if m.Config.SquashTimeout > 0 {
       var cancel context.CancelFunc
       ctx, cancel = context.WithTimeout(context.Background(), time.Duration(m.Config.SquashTimeout)*time.Second)
       defer cancel()
   } else {
       ctx = context.Background()
   }

   This would maintain backward compatibility and allow users to set reasonable limits if needed.

---

🐛 Potential Bugs & Issues

Critical Concern:

1. No cancellation mechanism: With context.Background(), there's no way to cancel a hung summarization request. If the LLM server becomes unresponsive, TmuxAI will hang indefinitely.

   • Impact: Users may need to force-quit the application (Ctrl+D) as reported in issue Squashing causes tmuxai to freeze #80
   • Recommendation: Pass through a cancellable context from the caller, or at least provide a way to interrupt the operation

2. HTTP client timeout: The underlying http.Client in AiClient (line 114 in ai_client.go:114) is created without a timeout:

   client: &http.Client{},

   This means the HTTP client has no default timeout either, which could cause indefinite hangs at the transport layer.

   • Recommendation: Set a reasonable HTTP client timeout:

   client: &http.Client{
       Timeout: 5 * time.Minute, // or make it configurable
   }

---

⚡ Performance Considerations

Observations:

1. Memory usage: Large context summarization could consume significant memory, especially with local LLMs processing long chat histories
2. UI responsiveness: The spinner continues running (line 95 in squash.go:95), but there's no progress indication for long-running operations

Suggestions:

• Consider adding periodic status updates or progress indicators for operations exceeding a certain duration (e.g., "Still processing... 30s elapsed")
• Monitor memory usage during large context operations

---

🔒 Security Concerns

Low Risk:

• No immediate security concerns identified
• The change doesn't introduce new attack vectors
• API keys and authentication remain properly handled

Considerations:

• Resource exhaustion: Without a timeout, a malicious or misconfigured LLM endpoint could cause resource exhaustion
• Denial of service: Users can't easily recover from a hung request without killing the process

---

🧪 Test Coverage

Current State:

• Existing test TestProcessUserMessage_ContextSquash (line 61 in process_message_test.go:61) validates that squashing is triggered when context size exceeds threshold
• However, there's no test coverage for the timeout removal or long-running summarization scenarios

Recommendations:

1. Add a test that simulates a slow LLM response to verify the behavior works as expected:

   func TestSummarizeChatHistory_SlowLLM(t *testing.T) {
       // Mock AI client that takes 60+ seconds to respond
       // Verify that summarization completes successfully
   }

2. Add a test for cancellation behavior:

   func TestSummarizeChatHistory_Cancellation(t *testing.T) {
       // Create a cancellable context
       // Cancel it mid-summarization
       // Verify proper cleanup and error handling
   }

---

📋 Overall Assessment

Rating: ⚠️ Needs Improvement

While this PR solves the immediate issue for local LLM users, it introduces a potential hang/freeze scenario with no recovery mechanism. The change trades one problem (timeout too short) for another (no timeout at all).

Recommended Actions:

1. High Priority: Implement context cancellation support to prevent indefinite hangs
2. High Priority: Add timeout configuration to the config file with 0 meaning "no timeout"
3. Medium Priority: Set a default HTTP client timeout
4. Medium Priority: Add test coverage for long-running operations
5. Low Priority: Consider adding progress indicators for long operations

Alternative Approach:
Instead of removing the timeout entirely, consider increasing the default timeout to something more reasonable (e.g., 5 minutes) and making it configurable. This provides safety while supporting slower local LLMs.

---

💡 Suggested Config Addition

# config.example.yaml
squash:
  # Timeout for summarization requests in seconds
  # Set to 0 for no timeout (useful for local LLMs with large contexts)
  # Default: 300 (5 minutes)
  timeout: 300

---

Great work on identifying and addressing the issue! With the recommended improvements, this PR will provide a robust solution for both cloud and local LLM users. 🚀

cc @alvinunreal

[claude]

Code Review: PR #85 - Remove timeout from squash summarization

Summary

This PR addresses issue #80 by removing the hard-coded 30-second timeout from the squash summarization functionality to support local LLM instances that may take longer to process large contexts.

---

✅ Positive Aspects

1. Clear Problem Resolution: The change directly addresses the reported issue where local LLMs (like those running via LM Studio) were timing out during squash operations.

2. Minimal, Focused Change: The code change is surgical - only modifying what's necessary in internal/squash.go:123-124 by removing the timeout and updating the comment to explain why.

3. Good Documentation:

   • The PR description clearly explains the change
   • The inline comment in the code explains the rationale
   • Added comprehensive installation instructions in README.md for users who want to test the latest fixes

4. Backward Compatible: This change doesn't break existing functionality for cloud-based LLMs - they will continue to work as before, just without an artificial timeout constraint.

---

🔍 Concerns & Recommendations

1. Potential for Indefinite Hangs (High Priority)

Issue: Using context.Background() with no timeout means the summarization request can hang indefinitely if:

• The local LLM crashes or becomes unresponsive
• There's a network issue with the API endpoint
• The server accepts the request but never responds

Impact: Users would have no way to recover except Ctrl+D to exit entirely (as mentioned in issue #80).

Recommendation: Consider a configurable timeout approach:

// In config.example.yaml, add:
squash_timeout: 300  # Timeout in seconds for squash operations, 0 for no timeout

// In internal/squash.go:
func (m *Manager) summarizeChatHistory(messages []ChatMessage) (string, error) {
    // ... existing code ...
    
    // Create a context for the summarization request
    var ctx context.Context
    var cancel context.CancelFunc
    
    timeout := m.Config.SquashTimeout
    if timeout > 0 {
        ctx, cancel = context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second)
        defer cancel()
    } else {
        ctx = context.Background()
    }
    
    summary, err := m.AiClient.GetResponseFromChatMessages(ctx, summarizationMessage, m.GetOpenRouterModel())
    // ... rest of code ...
}

This approach:

• Maintains the fix for local LLMs (set to 0 or high value)
• Provides protection against hangs for default users
• Gives users control over their experience

2. User Experience During Long Operations (Medium Priority)

Issue: The spinner that indicates processing (s := spinner.New(...)) provides feedback, but users have no way to cancel a long-running squash operation.

Recommendation: Consider implementing:

• A way to cancel the operation (listen for keyboard interrupt)
• Progress indication for very long operations
• A warning before squash if the context is extremely large

3. Test Coverage (Medium Priority)

Observation: While there's a test for needSquash() in /home/runner/work/tmuxai/tmuxai/internal/process_message_test.go:61-100, there don't appear to be tests that actually exercise the summarizeChatHistory() function or validate timeout behavior.

Recommendation: Add tests covering:

// Test that squash completes successfully with no timeout
func TestSummarizeChatHistory_NoTimeout(t *testing.T) { ... }

// Test context cancellation handling
func TestSummarizeChatHistory_ContextCancellation(t *testing.T) { ... }

4. README Documentation (Minor)

Observation: The new "Install from Master" section is helpful, but it's worth noting:

• The build command go build -o tmuxai should specify if any build tags or flags are needed
• Consider mentioning Go version requirements

Suggestion:

### Install from Master (Latest Development Version)

To install the latest development version directly from the master branch:

```bash
# Clone the repository
git clone https://github.com/alvinunreal/tmuxai.git
cd tmuxai

# Build from source (requires Go 1.21 or later)
go build -o tmuxai

---

🔒 Security Assessment

No significant security concerns identified. The change doesn't introduce new attack vectors:

• No user input is involved in the timeout change
• The API client still respects authentication and existing security measures
• The change is limited to internal timing behavior

---

⚡ Performance Considerations

Improved for Target Use Case:

• ✅ Local LLMs with large contexts can now complete without artificial timeout
• ⚠️ Could lead to extended wait times for users if their LLM is slow or unresponsive
• ⚠️ No upper bound on resource consumption during squash operations

Consideration: Monitor for issues where squash operations consume significant resources over extended periods.

---

📊 Overall Assessment

Status: ✅ Approve with Recommendations

This PR successfully solves the immediate problem reported in issue #80. The code change is clean, well-documented, and minimally invasive. However, I strongly recommend implementing a configurable timeout (as outlined above) to provide users with more control and prevent indefinite hangs in edge cases.

Suggested Path Forward:

1. Merge this PR to unblock users experiencing the timeout issue with local LLMs
2. Follow up with a configurable timeout feature in a subsequent PR
3. Add test coverage for the squash functionality

Priority: Medium-High (fixes a blocking issue for local LLM users)

---

🤖 Automated review by Claude Code

[claude]

Code Review for PR #85: Remove timeout from squash summarization

Summary

This PR addresses issue #80 by removing the hard-coded 30-second timeout in the summarizeChatHistory function, which was causing context deadline errors when using local LLM instances with large contexts.

---

✅ Positive Aspects

1. Problem Identification & Solution

• Directly addresses the reported issue: The fix correctly identifies that the 30-second timeout was too restrictive for local LLMs processing large contexts
• Minimal, focused change: Only modifies what's necessary (internal/squash.go:123-124)
• Clear intent: The comment explains why the timeout was removed

2. Code Quality

• Clean implementation: Simply replaces context.WithTimeout with context.Background()
• Maintains existing patterns: The AI client methods already accept context parameters
• Good documentation: Updated comment clearly states the reason for no timeout

3. Documentation

• Helpful addition: The new Install from Main section in README.md will help users test unreleased fixes
• Clear warning: Notes that the main branch may be less stable than releases

---

⚠️ Concerns & Suggestions

1. Security & Resource Management ⚠️

Issue: Using context.Background() creates an unbounded context with no timeout or cancellation mechanism.

Risks:

• If a local LLM hangs or becomes unresponsive, the application will wait indefinitely
• Network issues with remote endpoints could cause permanent hangs
• Resource exhaustion if multiple squash operations pile up
• No way for users to interrupt a stuck summarization

Recommendation: Add a configurable timeout (e.g., 5 minutes default) instead of removing it entirely.

2. Test Coverage ⚠️

Observation:

• No tests exist for the summarizeChatHistory function
• The existing test only verifies that squashing is triggered, not that it completes successfully

Recommendation: Add test coverage for summarization functionality including success, timeout, and error cases.

3. Error Handling

Current behavior: If summarization fails, error is logged but context size issue remains unresolved.

Suggestions:

• Consider retry logic for transient failures
• Provide user feedback when squashing fails
• Fall back to more aggressive context reduction

---

🎯 Recommendations

Priority 1 (High - Should address before merge):

1. Add configurable timeout instead of unbounded context
2. Add test coverage for summarization functionality
3. Improve error handling with fallback strategies

Priority 2 (Medium - Consider for follow-up PR):
4. Add user feedback for long-running operations
5. Add telemetry to track summarization duration

---

🎬 Conclusion

Overall Assessment: This PR successfully addresses the immediate issue reported in #80, but introduces potential for indefinite hangs.

Recommendation: Add a configurable timeout (e.g., 5 minutes default) instead of removing it entirely. This provides flexibility for local LLMs while preventing indefinite hangs.

Alternative: Merge as-is as a temporary fix, then open a follow-up issue for configurable timeouts.

---

🤖 Generated with Claude Code