feat: add session_title_generated event emission to CLI - #4590
Conversation
- Add SessionTitleGeneratedMessage type definition with sessionId, title, and timestamp fields - Update SessionTitleService to emit session_title_generated event after successfully updating session title - Add SessionTitleGeneratedStreamEvent to CliOutputParser for parsing the event from CLI output - Wire up communication between CLI and extension to forward the event - Add tests for parsing and emitting the new event type This event is emitted when a session title is generated, which typically happens after a session is created.
🦋 Changeset detectedLatest commit: 9eb185a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 2 Issues Found
| Severity | Issue | Location |
|---|---|---|
| WARNING | Duplicate type definition for SessionTitleGeneratedMessage |
SessionSyncService.ts:35-42 |
| SUGGESTION | Unused dependency onSessionTitleGenerated in SessionSyncService |
SessionSyncService.ts:65 |
Recommendation: Address the duplicate type definition to improve maintainability.
Review Details (8 files)
Files:
cli/src/cli.ts- ✅ Cleansrc/core/kilocode/agent-manager/CliOutputParser.ts- ✅ Cleansrc/core/kilocode/agent-manager/__tests__/CliOutputParser.spec.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionManager.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionSyncService.ts(2 issues)src/shared/kilocode/cli-sessions/core/SessionTitleService.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/__tests__/SessionTitleService.spec.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/extension/session-manager-utils.ts- ✅ Clean
Checked: Security, bugs, performance, error handling, code duplication
Overall: The implementation is well-structured and follows existing patterns. The event emission logic is correctly placed in SessionTitleService. Tests are comprehensive. The only issues are related to code organization (duplicate type definition and unused dependency).
…d dependency - Remove duplicate SessionTitleGeneratedMessage type from SessionSyncService - Import SessionTitleGeneratedMessage from SessionTitleService instead - Remove unused onSessionTitleGenerated dependency from SessionSyncService - Update SessionManager imports to reflect the changes - Add changeset for the PR
There was a problem hiding this comment.
✅ Previous Issues Addressed
The following issues from my previous review have been resolved:
- Duplicate type definition:
SessionTitleGeneratedMessageis now defined only inSessionTitleService.tsand imported where needed - Unused dependency:
onSessionTitleGeneratedwas removed fromSessionSyncServiceDependenciesinterface
No new issues found in the latest changes.
Review Details (9 files)
Files:
.changeset/session-title-generated-event.md- ✅ Cleancli/src/cli.ts- ✅ Cleansrc/core/kilocode/agent-manager/CliOutputParser.ts- ✅ Cleansrc/core/kilocode/agent-manager/__tests__/CliOutputParser.spec.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionManager.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionSyncService.ts- ✅ Clean (issues fixed)src/shared/kilocode/cli-sessions/core/SessionTitleService.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/__tests__/SessionTitleService.spec.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/extension/session-manager-utils.ts- ✅ Clean
Checked: Security, bugs, performance, error handling, code duplication, type consistency
Overall: The implementation is well-structured and follows existing patterns for event emission. Tests are comprehensive.
iscekic
left a comment
iscekic
left a comment
There was a problem hiding this comment.
Needs to fix the cli tests
There was a problem hiding this comment.
✅ No New Critical Issues
1 file reviewed with 1 minor suggestion | Confidence: 90% | Recommendation: Merge after addressing human reviewer feedback
| Severity | Issue | Location |
|---|---|---|
| SUGGESTION | Unused import | SessionSyncService.ts:7 |
Review Details (10 files)
Files:
.changeset/session-title-generated-event.md- ✅ Changeset (skipped)cli/src/cli.ts- ✅ Cleansrc/core/kilocode/agent-manager/CliOutputParser.ts- ✅ Cleansrc/core/kilocode/agent-manager/__tests__/CliOutputParser.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/SessionManager.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionSyncService.ts- 1 suggestionsrc/shared/kilocode/cli-sessions/core/SessionTitleService.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/__tests__/SessionManager.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/__tests__/SessionTitleService.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/extension/session-manager-utils.ts- ✅ Clean
Checked: Security, bugs, performance, error handling, unused code
Previous Issues: The duplicate type definition and unused dependency issues from the previous review have been resolved.
Note: Human reviewer @iscekic mentioned "Needs to fix the cli tests" - please ensure CI tests pass before merging.
There was a problem hiding this comment.
✅ No New Critical Issues
1 file reviewed with 1 minor suggestion | Confidence: 90% | Recommendation: Merge after addressing human reviewer feedback
| Severity | Issue | Location |
|---|---|---|
| SUGGESTION | Unused import | SessionSyncService.ts:7 |
Review Details (10 files)
Files:
.changeset/session-title-generated-event.md- ✅ Changeset (skipped)cli/src/cli.ts- ✅ Cleansrc/core/kilocode/agent-manager/CliOutputParser.ts- ✅ Cleansrc/core/kilocode/agent-manager/__tests__/CliOutputParser.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/SessionManager.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionSyncService.ts- 1 suggestionsrc/shared/kilocode/cli-sessions/core/SessionTitleService.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/__tests__/SessionManager.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/__tests__/SessionTitleService.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/extension/session-manager-utils.ts- ✅ Clean
Checked: Security, bugs, performance, error handling, unused code
Previous Issues: The duplicate type definition and unused dependency issues from the previous review have been resolved.
Note: Human reviewer @iscekic mentioned "Needs to fix the cli tests" - please ensure CI tests pass before merging.
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
There was a problem hiding this comment.
✅ No New Issues Found
9 files reviewed | Confidence: 92% | Recommendation: Merge after CI passes
Review Details
Files:
.changeset/session-title-generated-event.md- ✅ Changeset (skipped)cli/src/cli.ts- ✅ Cleansrc/core/kilocode/agent-manager/CliOutputParser.ts- ✅ Cleansrc/core/kilocode/agent-manager/__tests__/CliOutputParser.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/SessionManager.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/SessionTitleService.ts- ✅ Cleansrc/shared/kilocode/cli-sessions/core/__tests__/SessionManager.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/core/__tests__/SessionTitleService.spec.ts- ✅ Clean (test file)src/shared/kilocode/cli-sessions/extension/session-manager-utils.ts- ✅ Clean
Checked: Security, bugs, performance, error handling, type consistency
Implementation Quality:
- Type definition (
SessionTitleGeneratedMessage) is properly centralized inSessionTitleService.ts - Event emission follows established patterns for
session_createdandsession_synced - Parser correctly handles both with and without timestamp
- Tests are comprehensive
Previous Issues: All resolved (comments now outdated)
Note: Human reviewer @iscekic mentioned "Needs to fix the cli tests" - please ensure CI tests pass before merging.
2 participants
Summary
This PR adds the
session_title_generatedevent emission to the CLI, similar to howsession_createdandsession_syncedare currently emitted.Changes
Added event type definitions for
SessionTitleGeneratedMessagewith sessionId, title, and timestamp fields in:SessionSyncService.tsSessionTitleService.tsUpdated SessionTitleService to emit the
session_title_generatedevent after successfully updating the session title in theupdateTitlemethodAdded SessionTitleGeneratedStreamEvent to
CliOutputParser.tsfor parsing the event from CLI outputWired up communication between the CLI and extension to forward this event:
SessionManager.tsto accept and pass the callbackcli.tsto handle the event in JSON modesession-manager-utils.tsto log the event in the extensionAdded tests for the new event type:
session_title_generatedevents inCliOutputParser.spec.tsSessionTitleService.spec.tsBehavior
This event is emitted when the session title is generated, which typically happens after a session is created. The event includes:
sessionId: The ID of the sessiontitle: The generated titletimestamp: Unix timestamp when the event was emittedevent: The event type (session_title_generated)Testing
Added comprehensive tests for: