-
Notifications
You must be signed in to change notification settings - Fork 242
Concurrent agent support with tab view #1628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
I found 4 concurrency-related issues in the concurrent agent implementation. These are real bugs that could cause crashes or resource leaks:
- 1 HIGH severity: Double-close panic on programReady channel (latent, unlikely with current usage)
- 3 MEDIUM severity: Race conditions and cleanup synchronization issues
All issues are in the new pkg/tui/service/supervisor/supervisor.go file. The most critical is the missing synchronization in cleanup operations, which could leave resources in inconsistent states.
Key Findings:
- Unprotected channel close operation (potential panic)
- Goroutine spawning s.program access without synchronization
- Cleanup functions launched without WaitGroup or synchronization
These should be addressed before merge to ensure stability of the concurrent agents feature.
792e80c to
04c7b2f
Compare
|
/review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Code Review Summary
Reviewed the concurrent agent support implementation. Found 2 issues worth addressing:
Findings:
- 1 issue with error handling in cleanup goroutines
- 1 issue with shallow copying causing shared references
Both are medium severity but could cause issues in production under specific conditions.
04c7b2f to
12388ea
Compare
|
/review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Found 3 issues in the new concurrent agent support code that should be addressed:
- HIGH Severity: Goroutine leak if SetProgram() is never called (could leak all subscription goroutines)
- MEDIUM Severity: Unnecessary goroutine spawning in notifyTabsUpdated() with missing error handling
- MEDIUM Severity: Inconsistent cleanup pattern that may cause deadlock
All issues are in the new pkg/tui/service/supervisor/supervisor.go file. The rest of the PR looks good - the concurrent agent architecture is well-designed with proper locking and context cancellation patterns.
12388ea to
9c3ae7e
Compare
|
/review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review Summary
Found 4 issues in the concurrent agent implementation that should be addressed:
- 2 CONFIRMED bugs (configuration cloning and blocking I/O)
- 2 LIKELY coordination issues (cleanup goroutine patterns)
The concurrent agent architecture looks solid overall. The main concerns are around proper resource cleanup coordination and using appropriate contexts for I/O operations.
7205b41 to
033027b
Compare
…ions Signed-off-by: krissetto <chrisjpetito@gmail.com>
033027b to
20e3ead
Compare
Concurrent agents
Introduces an initial version of a new feature: concurrent agents.
This allows users to open multiple "tabs" with their current agent, even in separate working directories, and run them in parallel.
Includes:
The feature is gated behind the
CAGENT_EXPERIMENTAL_CONCURRENT_AGENTSenvironment variable. The rest of the TUI should remain as-is, i created a separate model strongly based off of the original TUI model to touch as little of the existing code as possible until we all test things and agree on the approachScreencast
Screencast.From.2026-02-08.16-37-05.mp4