feat: expand integrated terminal with named tabs, shell profiles, and links - #36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d637e93b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if (!terminal.exited) terminal.process.kill(); | ||
| const { pty, resolvedShellId } = spawnPty(projectDir, terminal.shellId); | ||
| terminal.process = pty; |
There was a problem hiding this comment.
When a running terminal is restarted, kill() schedules the old process's exit callback, but the terminal map is immediately updated to the new process under the same ID. The old handler then looks up that ID, marks the new terminal as exited, emits terminalExit, and causes subsequent writes and resizes to be ignored. Bind handlers to a specific PTY instance or dispose/ignore the old handlers before replacing the process.
Useful? React with 👍 / 👎.
| void rpc.request | ||
| .openFileIn({ projectDir, file: match.file, editorId: preferredEditorId, line: match.line }) |
There was a problem hiding this comment.
After a user runs cd subdirectory, tools commonly print paths relative to that new directory, but every matched link is sent with the original projectDir. openFileIn therefore resolves something like src/main.ts:42 to <project>/src/main.ts rather than <project>/subdirectory/src/main.ts, opening the wrong file or failing whenever the terminal is not at the project root.
Useful? React with 👍 / 👎.
| createTerminal( | ||
| projectDir, | ||
| undefined, | ||
| undefined, |
There was a problem hiding this comment.
When a user selects and persists Git Bash, WSL, or another profile, reopening the dock with no existing terminals still passes undefined as the shell ID, so terminalEnsure always starts the PowerShell fallback. The saved preference currently affects only subsequently created splits, making it ineffective for the normal single-terminal workflow.
Useful? React with 👍 / 👎.
| if (!session.exited) { | ||
| return <span className="size-1.5 shrink-0 rounded-full bg-emerald-500" aria-hidden="true" />; |
There was a problem hiding this comment.
The running status dot introduces the raw Tailwind color bg-emerald-500, so it bypasses the project's semantic theme tokens and can drift from the established visual system. Add or reuse an appropriate semantic status token instead.
AGENTS.md reference: AGENTS.md:L132-L135
Useful? React with 👍 / 👎.
|
|
||
| const FILE_LINE_RE = /((?:[A-Za-z]:)?(?:[\w.-]+[\\/])*[\w.-]+\.[A-Za-z0-9]+):(\d+)(?::(\d+))?/g; | ||
| const LOCALHOST_RE = /https?:\/\/(?:localhost|127\.0\.0\.1|\[::1\])(?::\d+)?(?:\/[^\s"'<>)\]]*)?/g; | ||
|
|
There was a problem hiding this comment.
For common Windows paths containing spaces, such as C:\Users\Jane Doe\repo\src\main.ts:42, this regex starts matching only after the last space and returns a suffix such as Doe\repo\src\main.ts or main.ts. Clicking the resulting link resolves that suffix beneath projectDir, so absolute compiler and stack-trace paths open the wrong file or fail.
Useful? React with 👍 / 👎.
| range: { | ||
| start: { x: match.start + 1, y: bufferLineNumber }, | ||
| end: { x: match.end, y: bufferLineNumber }, | ||
| }, |
There was a problem hiding this comment.
match.start and match.end are JavaScript string offsets, while xterm link ranges use terminal cell columns. If output before a link contains a wide or combining character—for example 界 src/main.ts:42—the computed range begins or ends on the wrong cells, producing incorrect highlighting and unreliable activation. Derive the coordinates from buffer-cell widths rather than string indices.
Useful? React with 👍 / 👎.
| <button | ||
| type="button" | ||
| className="min-w-0 flex-1 truncate text-left text-xs font-medium" | ||
| onDoubleClick={() => setEditingName(true)} | ||
| title={session.exited ? `${session.name} — exited (code ${session.exitCode ?? "unknown"})` : session.name} |
There was a problem hiding this comment.
The terminal name is exposed as a focusable button, but its only action is registered through onDoubleClick. Keyboard activation of a button produces a click, not a double-click, so pressing Enter or Space does nothing and leaves keyboard users on an inert control instead of entering rename mode.
Useful? React with 👍 / 👎.
| void rpc.request | ||
| .openFileIn({ projectDir, file: match.file, editorId: preferredEditorId, line: match.line }) |
There was a problem hiding this comment.
The parser retains the optional column from links such as src/main.ts:42:17, but activation sends only match.line through RPC. Editors with column-aware goto syntax therefore always open at the beginning of line 42 instead of the reported error location, discarding information that the new link matcher explicitly recognized.
Useful? React with 👍 / 👎.
| async function restartSplit(terminalId: string) { | ||
| try { | ||
| const { terminal } = await rpc.request.terminalRestart({ projectDir, terminalId }); | ||
| setTerminals((current) => current.map((session) => (session.id === terminalId ? terminal : session))); |
There was a problem hiding this comment.
When the desktop and a browser are viewing the same terminal, a restart updates only the renderer that issued this request. Other viewers receive new terminal data but no restart metadata, so they retain the old scrollback and status—an exited terminal can remain labeled exited while accepting input again. Emit a restart/state event so every connected renderer clears its surface and replaces the session metadata.
Useful? React with 👍 / 👎.
|
|
||
| const FILE_LINE_RE = /((?:[A-Za-z]:)?(?:[\w.-]+[\\/])*[\w.-]+\.[A-Za-z0-9]+):(\d+)(?::(\d+))?/g; | ||
| const LOCALHOST_RE = /https?:\/\/(?:localhost|127\.0\.0\.1|\[::1\])(?::\d+)?(?:\/[^\s"'<>)\]]*)?/g; | ||
|
|
There was a problem hiding this comment.
The new WSL and Git Bash profiles can emit absolute paths such as /mnt/c/project/src/main.ts:42, but this expression omits the leading slash and returns mnt/c/project/src/main.ts. The renderer then treats that as project-relative and opens a nonexistent nested path, so absolute stack-trace links from two of the newly supported shells do not work.
Useful? React with 👍 / 👎.
1 participant
Summary
file:line[:col]links in terminal output that open the file in the user's preferred editor at that line (VS Code family via-g, JetBrains via--line)http(s)://localhost/127.0.0.1links that open in the default browserScope notes
AGENTS.md's "integrated terminals" carve-out.openFileIn/openExternalIPC and the existing editor-detection pattern rather than adding new subsystems.Test plan
bun run typecheck(root)bun run test(root) — 158 passbun run build(root)AGENTS.md)