-
Notifications
You must be signed in to change notification settings - Fork 17
ECHO-335 Add terminal keeper #209
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
Conversation
…anced terminal management
…or cleaner terminal management.
WalkthroughThis change updates the devcontainer configuration to include the "nguyenngoclong.terminal-keeper" VSCode extension and introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant VSCode
participant Terminal Keeper Extension
Developer->>VSCode: Open devcontainer
VSCode->>VSCode: Load extensions (including terminal-keeper)
Developer->>VSCode: Start terminal session (sessions.json)
VSCode->>Terminal Keeper Extension: Parse sessions.json
Terminal Keeper Extension->>VSCode: Create terminal groups
loop For each terminal group
Terminal Keeper Extension->>VSCode: Create terminal(s) with auto-execute commands
end
VSCode->>Developer: Ready terminals with running processes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. LGTM. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@spashii i'll update the docs if we go forward with this solution after testing |
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
echo/.devcontainer/devcontainer.json(1 hunks)echo/.vscode/sessions.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ussaama
PR: Dembrane/echo#205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.663Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cursor BugBot
- GitHub Check: ci-check-frontend
- GitHub Check: ci-check-server
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
echo/.vscode/sessions.json (1)
17-45: Nested array shape – double-check against Terminal-Keeper specThe file mixes single objects and arrays to express split groups. The v10 spec expects:
[ Terminal | { split: [ Terminal, … ] }, … ]Here we directly embed
[ {…}, {…} ]without thesplitkey (lines 17-45). It might still parse, but it’s undocumented. Verify in VS Code: if groups don’t split as intended, wrap them with a{ "split": [...] }.
…dd icons for various terminal sessions for improved visual organization.
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
echo/.vscode/sessions.json (2)
2-2: Vendor the JSON schema locally for offline resiliencePointing
$schemaat a CDN makes validation brittle when you’re on a plane or the CDN hiccups. Drop the file under.vscode/schema/terminal-keeper.jsonand reference it relatively.
14-15: Hard-coded.venvpath → use a wrapper scriptFour separate terminals break if the user’s venv isn’t named
.venv(Poetry, Conda, etc.). Stick the activation logic inserver/activate_and_run.shand call that instead. Same advice as the previous review.Also applies to: 24-26, 34-36, 44-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
echo/.vscode/sessions.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ussaama
PR: Dembrane/echo#205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.663Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
…d terminal management.
|
@ussaama in this branch remove the "restore-terminals" from the .devcontainer |
…ons from VSCode settings for cleaner setup.
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.
Bug: Mixed Array Types in Session Data
The default session array in sessions.json contains inconsistent element types, mixing a single terminal object ('server') with arrays of terminal objects (e.g., 'workers', 'scheduler'). This mixed structure will likely cause parsing errors in the terminal-keeper extension. Furthermore, the 'scheduler' terminal is now grouped with 'workers' and 'workers-cpu', altering its intended separate terminal organization.
echo/.vscode/sessions.json#L6-L70
echo/echo/.vscode/sessions.json
Lines 6 to 70 in 3847725
| "sessions": { | |
| "default": [ | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "server", | |
| "icon": "server", | |
| "commands": [ | |
| "cd server", | |
| "source .venv/bin/activate", | |
| "./run.sh" | |
| ] | |
| }, | |
| [ | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "workers", | |
| "icon": "gear", | |
| "commands": [ | |
| "cd server", | |
| "source .venv/bin/activate", | |
| "./run-worker.sh" | |
| ] | |
| }, | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "workers-cpu", | |
| "icon": "gear", | |
| "commands": [ | |
| "cd server", | |
| "source .venv/bin/activate", | |
| "./run-worker-cpu.sh" | |
| ] | |
| }, | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "scheduler", | |
| "icon": "clock", | |
| "commands": [ | |
| "cd server", | |
| "source .venv/bin/activate", | |
| "./run-scheduler.sh" | |
| ] | |
| } | |
| ], | |
| [ | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "admin-dashboard", | |
| "icon": "browser", | |
| "commands": [ | |
| "cd frontend", | |
| "pnpm run dev" | |
| ] | |
| }, | |
| { | |
| "autoExecuteCommands": true, | |
| "name": "participant-portal", | |
| "icon": "browser", | |
| "commands": [ | |
| "cd frontend", | |
| "pnpm run participant:dev" | |
| ] | |
| } | |
| ] | |
| ] |
Was this report helpful? Give feedback by reacting with 👍 or 👎
* Update devcontainer.json to include terminal-keeper extension for enhanced terminal management * Remove activateOnStartup setting from VSCode sessions configuration for cleaner terminal management. * Update VSCode sessions configuration to change theme to 'tribe' and add icons for various terminal sessions for improved visual organization. * Update devcontainer.json to add terminal-keeper extension for enhanced terminal management. * Remove "restore-terminals" extension and related terminal configurations from VSCode settings for cleaner setup.
Summary by CodeRabbit