Add local servers to the thread context panel#5
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces terminal session listing with external server discovery, session metadata tracking, and a local-server management UI. It extends the terminal contract with metadata and list types, implements platform-specific external server discovery for Windows and POSIX systems, adds repository URL extraction to git status, and wires a new chat sidebar section for managing local-run project scripts and viewing repository links. ChangesTerminal List Feature with Metadata and Local Servers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/server/src/terminal/Layers/Manager.ts`:
- Around line 867-875: The close method currently calls externalProcessKiller
for any terminalId parsed as external:<pid> without authorization; modify close
(and/or externalPidFromTerminalId) to validate that the resolved externalPid is
owned/allowed for the current thread/caller/project before killing it: after
decoding input via decodeTerminalCloseInput and inside runWithThreadLock use the
threadId (and any caller/project context available) to look up a trusted
registry/mapping of discovered external PIDs (or check an ownership set stored
by the terminal manager) and only call externalProcessKiller(externalPid) if the
PID is present and belongs to that thread/caller; otherwise reject/throw and do
not call externalProcessKiller. Ensure you reference/validate via the same
identifiers used elsewhere (threadId, terminalId) so arbitrary external:<pid>
requests cannot kill unrelated processes.
- Around line 349-357: commandMatchesProjectRoot currently uses
normalizedCommandLine.includes(root) which allows substring collisions (e.g.
"/repo" matching "/repo2"); change the matching to enforce path-boundary checks:
normalize both commandLine and each project root (use "/" separators and ensure
roots end with a trailing "/"), then check for either exact equality or that
normalizedCommandLine has the root followed by a path separator or end-of-string
(or split normalizedCommandLine into path segments and compare segment-wise).
Update the logic inside commandMatchesProjectRoot to use this stricter
comparison instead of includes to avoid false positives.
In `@apps/server/src/wsServer.ts`:
- Around line 2599-2602: The terminalList websocket handler
(WS_METHODS.terminalList -> terminalManager.list) is missing the terminal
permission gating and therefore falls back to orchestration:read; update the
permission check mapping used by the WS method dispatcher (the structure that
defines requiredRemoteAccessScope for each WS_METHODS value) to include
WS_METHODS.terminalList and require the "terminal:operate" scope, or add an
explicit permission check before calling terminalManager.list that enforces
"terminal:operate"; ensure you reference WS_METHODS.terminalList and
terminalManager.list so the route is protected by the same terminal permission
logic as other terminal methods.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 554-567: normalizeLocalServerPath currently forces lowercase which
can break case-sensitive filesystems; remove the final .toLowerCase() so the
function returns the normalized path with original case (keep slash
normalization, trim, and trailing slash removal), and ensure
localServerPathMatches uses those case-preserved normalized values for equality
and startsWith checks (i.e., compare normalizedCandidate === normalizedRoot and
normalizedCandidate.startsWith(`${normalizedRoot}/`) without lowercasing).
- Around line 705-708: The current sshLikeMatch regex in ChatView.tsx is too
permissive and treats ssh://git@... with ports as scp-style remotes; update the
pattern used for sshLikeMatch to only match scp-style SSH remotes (e.g.
git@host:org/repo.git) by removing the ssh://git@ alternative and using a
pattern like ^git@([^/:]+):([^?#]+?)(?:\.git)?\/?$ so host and path are parsed
correctly; keep the existing replacement logic that returns
`https://${sshLikeMatch[1]}/${sshLikeMatch[2].replace(/\.git$/i, "")}` and
ensure any ssh://... cases are left to the other parsing path instead of this
scp-only branch.
- Around line 7990-8014: The query is polling regardless of the popover state;
update the terminalSessionsQuery to stop polling when the local-servers popover
is closed by adding the popover open state to its enabled condition and query
identity and only applying refetchInterval when open. Concretely: include the
popover boolean (e.g., localServersPopoverOpen or isLocalServersOpen) in the
queryKey and change enabled to Boolean(activeThreadId &&
localServersPopoverOpen), and set refetchInterval to LOCAL_SERVER_REFRESH_MS
only when localServersPopoverOpen is true (otherwise disable or set to false) so
readNativeApi / api.terminal.list() is only called while the popover is open.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 217576bf-1313-42f1-9528-53035d3f9ef0
📒 Files selected for processing (14)
apps/server/src/git/Layers/GitManager.tsapps/server/src/terminal/Layers/Manager.test.tsapps/server/src/terminal/Layers/Manager.tsapps/server/src/terminal/Services/Manager.tsapps/server/src/wsServer.test.tsapps/server/src/wsServer.tsapps/web/src/components/ChatView.tsxapps/web/src/wsNativeApi.tsapps/web/src/wsTransport.tspackages/contracts/src/git.tspackages/contracts/src/ipc.tspackages/contracts/src/terminal.test.tspackages/contracts/src/terminal.tspackages/contracts/src/ws.ts
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.
Problem statement
The thread context panel had grown into a useful place for project state, but it was still missing two practical pieces during day-to-day work:
A few follow-on issues surfaced while building this:
What changed
This PR turns the thread context panel into a more complete project control surface.
Local serverssection to the panelImplementation notes
terminal.listremote.origin.urlthrough the existing git status path so the repository row does not require a second queryVerification
bun run typecheckbun run lintnode ../../scripts/run-vitest.cjs run src/terminal/Layers/Manager.test.ts -t "external project-local"Summary by CodeRabbit