Fix tracked CLI session naming#675
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 30, 2026 16:47
📝 Walkthrough
WalkthroughThree parallel tracks: (1) ChangesCLI Title Sanitization and OSC Window-Title Adoption
FilesWorkbench Read-Only Edit Overrides and Recent-File Re-scoping
agentChatService readTranscript Non-Chat Session Guard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/main/services/pty/ptyService.test.ts (1)
3458-3520: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a split-chunk OSC regression test.
runtimeWindowTitleScanBufferis the new tricky part here, but these assertions only cover one-chunk\x1b]0;...\x07payloads. A two-write case would protect the buffering logic that motivated the new entry state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/pty/ptyService.test.ts` around lines 3458 - 3520, The new OSC handling in runtimeWindowTitleScanBuffer is only covered by single-chunk payloads, so add a regression test in ptyService.test.ts that emits the title sequence across two data writes. Use the existing ptyService/createHarness setup and the Claude runtime title flow to verify the split escape sequence is buffered correctly and still updates sessionService.get/updateMeta with the parsed title once the second chunk arrives.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/files/v2/FilesWorkbench.tsx`:
- Around line 72-76: The edit override check in canEditWorkspace is keyed only
by workspace ID, which can leak opt-in across project switches when IDs are
reused. Update the override lookup to use the same projectRootPath-scoped
composite key used elsewhere in FilesWorkbench, and propagate that keying
through the related edit-override call sites in this component so overrides are
tied to the current project root rather than just the workspace id.
- Around line 147-150: The recent-files key in FilesWorkbench should distinguish
attached workspaces from lane-scoped workspaces instead of falling through to
workspace.laneId ?? globalLaneId. Update the recentSessionKey logic in
FilesWorkbench so the filesSessionKey input uses workspace.id for attached
workspaces (similar to the external branch) and only uses laneId/globalLaneId
for the appropriate workspace kinds. This keeps attached workspace recents
scoped to the selected workspace rather than the selected lane.
In `@apps/desktop/src/shared/cliLaunch.ts`:
- Around line 125-140: The lane-path parser in cliLaunch.ts is incorrectly
treating the user task as part of the prelude when there is no blank line after
the ADE lane path. Update the trimming logic around the loop that advances past
the lane path so it stops at the first non-empty line after the path instead of
requiring an empty separator, and ensure the fallback to raw.trim() is only used
when no task content remains. Keep the fix localized to the remainder extraction
in the helper that derives the goal/title.
---
Nitpick comments:
In `@apps/desktop/src/main/services/pty/ptyService.test.ts`:
- Around line 3458-3520: The new OSC handling in runtimeWindowTitleScanBuffer is
only covered by single-chunk payloads, so add a regression test in
ptyService.test.ts that emits the title sequence across two data writes. Use the
existing ptyService/createHarness setup and the Claude runtime title flow to
verify the split escape sequence is buffered correctly and still updates
sessionService.get/updateMeta with the parsed title once the second chunk
arrives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 444beb95-58b2-4f4b-a299-8d4ec3ffc13b
⛔ Files ignored due to path filters (2)
docs/features/chat/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**
📒 Files selected for processing (11)
apps/ade-cli/src/bootstrap.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/renderer/components/files/v2/EditorGroups.tsxapps/desktop/src/renderer/components/files/v2/FilesWorkbench.test.tsxapps/desktop/src/renderer/components/files/v2/FilesWorkbench.tsxapps/desktop/src/renderer/components/files/v2/editorGroupsStore.tsapps/desktop/src/renderer/components/terminals/cliLaunch.test.tsapps/desktop/src/shared/cliLaunch.ts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR fixes CLI session naming accuracy in ADE by stripping ADE-specific guidance wrappers from prompts before deriving the session title/goal, and adds OSC window title adoption as a fallback when AI title generation is unavailable.
cliLaunch.ts: NewunwrapAdeGuidancePromptForTitleandstripAdeLaneDirectiveForTitlehelpers extract the actual user task from ADE runtime guidance, so lane/worktree directives don't appear as session names.ptyService.ts: NewadoptCliRuntimeWindowTitlehandler reads OSC\\x1b]0;/\\x1b]2;sequences from PTY output and applies the sanitized title to the session when AI generation is not active.runtimeWindowTitleScanBuffertracks partial escape sequences across data chunks.agentChatService.ts:readTranscriptnow early-exits for blank or non-chat session IDs, preventing accidental transcript reads for terminal sessions.FilesWorkbench.tsx: Recent-file records are now scoped per-workspace rather than per-project, and read-only-by-default workspaces expose an "Enable editing" session-local override.Confidence Score: 5/5
Safe to merge; all changed paths are additive or guarded, tests cover both the happy path and the AI-suppression path for the new OSC title adoption.
The ADE guidance unwrapping, OSC title adoption, and recent-file scoping changes are all well-tested. The only observable behavioral quirk — OSC titles replacing user-prompt-derived names in guest/offline mode — is exercised by the test suite and appears intentional. No data-loss, crash, or auth-boundary concerns were found.
apps/desktop/src/shared/cliLaunch.ts — the
looksLikeAdeGuidancesecond arm and the empty-remainder path instripAdeLaneDirectiveForTitle(already noted in prior threads). apps/desktop/src/main/services/pty/ptyService.ts —adoptCliRuntimeWindowTitlehas no placeholder guard before overwriting.Important Files Changed
looksLikeAdeGuidancedetection (covered by prior threads); core extraction logic is sound.adoptCliRuntimeWindowTitle) with a rolling 2 KB buffer; guard logic correctly skips adoption when AI title generation is active.readTranscriptfor blank session IDs and non-chat tool types; logic is correct.recentScopeIdForWorkspace; session-local edit override accumulates without cleanup but is bounded to component lifetime.workspacesprop; refactored group iteration avoids!non-null assertion onprops.state.groups[id].aiIntegrationServiceandprojectConfigService(already created earlier in the bootstrap) tocreatePtyService, enabling AI title generation in the CLI runtime.LegacyEditorTabtype to replace the inline cast inupgradeLegacySession; no behavioral change.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[PTY data received] --> B[appendRecentOutput] A --> C[adoptCliRuntimeWindowTitle] A --> D[writeTranscript / feedTerminalSnapshot / updatePreview] C --> E{CLI_USER_TITLE_TOOL_TYPES?} E -- no --> Z[skip] E -- yes --> F{aiIntegrationService\nnon-guest &\nisTitleGenerationEnabled?} F -- yes --> Z F -- no --> G[extractLatestOscWindowTitle] G --> H{title found?} H -- no --> Z H -- yes --> I{manuallyNamed?} I -- yes --> Z I -- no --> J[sessionService.updateMeta\ntitle = OSC title] K[User input write] --> L[tryCliUserTitleFromWrite] L --> M{seed length OK &\nnot slash cmd?} M -- no --> Z M -- yes --> N[sanitizeTrackedCliPromptSeed] N --> O[unwrapAdeGuidancePromptForTitle] O --> P{ADE guidance?} P -- yes --> Q[extract after 'User prompt:'\nstripAdeLaneDirective] P -- no --> R[stripAdeLaneDirective] Q --> S[trackedCliTitleFromPromptSeed] R --> S S --> T{placeholder title?} T -- yes --> U[sessionService.updateMeta\ntitle = derived title] T -- no --> V{AI available?} V -- yes --> W[scheduleAiTitle] V -- no --> Z%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[PTY data received] --> B[appendRecentOutput] A --> C[adoptCliRuntimeWindowTitle] A --> D[writeTranscript / feedTerminalSnapshot / updatePreview] C --> E{CLI_USER_TITLE_TOOL_TYPES?} E -- no --> Z[skip] E -- yes --> F{aiIntegrationService\nnon-guest &\nisTitleGenerationEnabled?} F -- yes --> Z F -- no --> G[extractLatestOscWindowTitle] G --> H{title found?} H -- no --> Z H -- yes --> I{manuallyNamed?} I -- yes --> Z I -- no --> J[sessionService.updateMeta\ntitle = OSC title] K[User input write] --> L[tryCliUserTitleFromWrite] L --> M{seed length OK &\nnot slash cmd?} M -- no --> Z M -- yes --> N[sanitizeTrackedCliPromptSeed] N --> O[unwrapAdeGuidancePromptForTitle] O --> P{ADE guidance?} P -- yes --> Q[extract after 'User prompt:'\nstripAdeLaneDirective] P -- no --> R[stripAdeLaneDirective] Q --> S[trackedCliTitleFromPromptSeed] R --> S S --> T{placeholder title?} T -- yes --> U[sessionService.updateMeta\ntitle = derived title] T -- no --> V{AI available?} V -- yes --> W[scheduleAiTitle] V -- no --> ZReviews (2): Last reviewed commit: "Address CLI naming review feedback" | Re-trigger Greptile