feat(session-manager): add session_rename tool#1258
Open
moha-abdi wants to merge 13 commits intocode-yeongyu:devfrom
Open
feat(session-manager): add session_rename tool#1258moha-abdi wants to merge 13 commits intocode-yeongyu:devfrom
moha-abdi wants to merge 13 commits intocode-yeongyu:devfrom
Conversation
…DESCRIPTION constant
…meSession storage functions
…ssion storage functions
- Updated SESSION_RENAME_DESCRIPTION to instruct LLM to generate titles - When user says 'rename this session' without title, LLM analyzes context - Zero overhead: LLM generates title before calling tool - Max 80 char descriptive titles based on conversation themes
- Reject empty titles at tool level with clear error message - Storage layer preserves existing title if empty string passed - Made session_id optional, defaults to current session - Updated description: title cannot be empty Fixes OpenCode TUI crash: undefined is not an object (evaluating 'str3.length')
- Test empty title validation instead of missing parameter - Removes 'as any' anti-pattern per contributing guidelines
- Extract title lookup to dedicated getSessionTitle function - Reuse findSessionMetadataPath instead of duplicating logic - Convert explanatory comments to BDD format (#given, #then) - Eliminates problematic catch block with comment
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 2/5
- Unvalidated
sessionIDused to build metadata paths insrc/tools/session-manager/storage.tsallows path traversal outside SESSION_STORAGE, a concrete security and data-integrity risk. - The test in
src/tools/session-manager/tools.test.tsperforms real filesystem writes viasession_rename, which can mutate local session metadata and cause flaky or corrupting test runs. - Given the security-impacting path traversal risk (severity 7/10), this change is high risk without additional validation/guards.
- Pay close attention to
src/tools/session-manager/storage.tsandsrc/tools/session-manager/tools.test.ts- path traversal protection and test isolation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/tools/session-manager/tools.test.ts">
<violation number="1" location="src/tools/session-manager/tools.test.ts:130">
P2: Test invokes session_rename which performs real filesystem writes without isolation; if a matching session exists locally, the test mutates real session metadata and can be flaky or corrupt developer data.</violation>
</file>
<file name="src/tools/session-manager/storage.ts">
<violation number="1" location="src/tools/session-manager/storage.ts:265">
P1: Unvalidated sessionID is used to build metadata paths, enabling path traversal outside SESSION_STORAGE when user-provided session_id contains "../" or path separators.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (!projectDir.isDirectory()) continue | ||
|
|
||
| const projectPath = join(SESSION_STORAGE, projectDir.name) | ||
| const sessionFile = `${sessionID}.json` |
There was a problem hiding this comment.
P1: Unvalidated sessionID is used to build metadata paths, enabling path traversal outside SESSION_STORAGE when user-provided session_id contains "../" or path separators.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/session-manager/storage.ts, line 265:
<comment>Unvalidated sessionID is used to build metadata paths, enabling path traversal outside SESSION_STORAGE when user-provided session_id contains "../" or path separators.</comment>
<file context>
@@ -232,7 +247,49 @@ export async function getSessionInfo(sessionID: string): Promise<SessionInfo | n
+ if (!projectDir.isDirectory()) continue
+
+ const projectPath = join(SESSION_STORAGE, projectDir.name)
+ const sessionFile = `${sessionID}.json`
+ const sessionPath = join(projectPath, sessionFile)
+
</file context>
| const args = { session_id: "ses_nonexistent", new_title: "New Title" } | ||
|
|
||
| //#when executing rename | ||
| const result = await session_rename.execute(args, mockContext) |
There was a problem hiding this comment.
P2: Test invokes session_rename which performs real filesystem writes without isolation; if a matching session exists locally, the test mutates real session metadata and can be flaky or corrupt developer data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/session-manager/tools.test.ts, line 130:
<comment>Test invokes session_rename which performs real filesystem writes without isolation; if a matching session exists locally, the test mutates real session metadata and can be flaky or corrupt developer data.</comment>
<file context>
@@ -121,4 +121,62 @@ describe("session-manager tools", () => {
+ const args = { session_id: "ses_nonexistent", new_title: "New Title" }
+
+ //#when executing rename
+ const result = await session_rename.execute(args, mockContext)
+
+ //#then returns error message
</file context>
…ataPath
- Replace Bun.file() check with readdirSync({ withFileTypes: true })
- Use .isDirectory() to properly filter directories
- Aligns with existing codebase pattern
bbf3b06 to
328f3de
Compare
The mock.module in storage.test.ts was missing the new export, causing CI failures when other test files tried to import from the polluted module cache.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
session_renametool to the session-manager.The session-manager already provides
session_list,session_read,session_search, andsession_infotools for reading session data. However, renaming a session currently requires manual intervention through the TUI (Ctrl+Rin session list). This addssession_renameto complete the toolset, allowing the LLM to rename sessions programmatically without user interaction.What This Does
session_renametool that updates thetitlefield in session metadata JSON filessession_idis not providedsession_listandsession_infooutputsFiles Changed
types.tsSessionRenameArgsinterfaceconstants.tsSESSION_RENAME_DESCRIPTIONwith LLM instructionsstorage.tsfindSessionMetadataPath,renameSession,getSessionTitletools.tssession_renametoolutils.tsformatSessionListandformatSessionInfoindex.tssession_renameinbuiltinToolsTesting
Full TDD approach with BDD comments.
Usage
Summary by cubic
Add a session_rename tool to rename sessions programmatically and surface titles in session_list and session_info. It defaults to the current session, validates titles to prevent TUI crashes, and guides the model to auto-generate a descriptive title when the user doesn’t provide one.
New Features
Bug Fixes
Written for commit a326de8. Summary will update on new commits.