Track terminal listening ports in activity events and thread state#43
Open
juliusmarminge wants to merge 16 commits into
Open
Track terminal listening ports in activity events and thread state#43juliusmarminge wants to merge 16 commits into
juliusmarminge wants to merge 16 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a cross-platform subprocess inspector and web-port probing to TerminalManager, surfaces per-terminal running port lists in activity events, and propagates those fields through contracts, server modules, web state, persistence, UI, and tests. Changes
Sequence DiagramsequenceDiagram
participant TM as Server:TerminalManager
participant INS as Inspector (process + web)
participant CONTRACT as Contracts/Validator
participant STORE as Web:Store
participant UI as Sidebar
TM->>INS: inspectSubprocess(terminalPid)
INS->>INS: collect PIDs (children/family) and check listening ports
INS->>INS: probe HTTP on detected ports (webPortInspector)
INS-->>TM: { hasRunningSubprocess, runningPorts[] }
TM->>CONTRACT: emit TerminalActivityEvent (includes runningPorts)
CONTRACT-->>STORE: validated event
STORE->>STORE: normalizeRunningPorts(runningPorts) and update runningTerminalPorts[terminalId]
STORE-->>UI: state change (thread.runningTerminalPorts)
UI->>UI: compute portsLabel / primaryWebPort and render clickable UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Track terminal web listening ports in activity events and surface a primary port in Sidebar and ThreadTerminalDrawer using TTL-cached probes (DEFAULT_WEB_PORT_PROBE_TTL_MS)Add 📍Where to StartStart with Macroscope summarized d377e56. |
Greptile SummaryExtends terminal activity tracking to detect and expose web server ports running in terminal subprocesses. The implementation polls subprocess state every second, uses platform-specific tools ( Major changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant TM as TerminalManager
participant PI as ProcessInspector
participant POSIX as POSIX/Win32
participant WP as WebPortInspector
participant Store as Web Store
participant UI as Sidebar UI
Note over TM: Terminal starts running
TM->>TM: Start subprocess polling (1s interval)
loop Every 1 second
TM->>PI: inspectSubprocess(terminalPid)
PI->>POSIX: collectProcessFamilyPids(pid)
POSIX-->>PI: [childPids]
PI->>POSIX: checkListeningPorts(childPids)
POSIX-->>PI: [rawPorts]
PI-->>TM: {hasRunningSubprocess, runningPorts}
TM->>TM: normalizeRunningPorts(ports)
loop For each detected port
TM->>WP: inspectWebPort(port)
Note over WP: Check cache (10s TTL)
WP->>WP: HTTP GET localhost:port/
WP-->>TM: isWeb (true if not 404)
end
TM->>TM: Filter to web ports only
TM->>Store: emit activity event{runningPorts}
Store->>Store: Update runningTerminalPorts map
Store->>UI: State change
UI->>UI: Show port badges with web icon
end
Note over TM: Terminal exits
TM->>TM: Clear runningSubprocessPorts
TM->>Store: emit exited event
Store->>Store: Remove from runningTerminalPorts
Last reviewed commit: c11027b |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/terminalManager.ts (1)
193-248:⚠️ Potential issue | 🟠 MajorWindows process collection only detects direct children—nested listeners spawned by npm/pnpm will be missed.
The Windows implementation queries only direct children via
Get-CimInstance Win32_Process -Filter "ParentProcessId = ${terminalPid}", while the POSIX equivalent recursively builds the full descendant tree (lines 276–288). When a dev server runs as a grandchild of the shell (e.g., npm → node → dev-server), the Windows path fails to detect its listening port. Consider implementing recursive process tree collection for Windows, similar to the POSIX approach.
🤖 Fix all issues with AI agents
In `@apps/web/src/store.ts`:
- Around line 706-734: The reducer mutates and reads action.event.runningPorts
without ensuring events were validated by the terminalEventSchema;
validate/parses incoming socket events through the Zod terminalEventSchema
before dispatch so defaults (like runningPorts defaulting to []) and types are
enforced. Update the WebSocket/event handler (e.g., in App.tsx or
wsNativeApi.ts) to call terminalEventSchema.parse or safeParse on the raw
message and only dispatch the parsed value to the reducer (which includes
normalizeThreadTerminals/runningTerminalPorts logic), and handle parse failures
(log/ignore) so the reducer never receives unvalidated events.
🧹 Nitpick comments (1)
apps/server/src/terminalManager.ts (1)
142-191: Usesort()instead oftoSorted()for runtime compatibility.The TypeScript config targets ES2023, but the project has no explicit Node.js version requirement in
package.jsonor deployment config.toSorted()requires Node 20+; without a guaranteed minimum version, this will cause runtime failures on older LTS releases. Replace withsort()for broader compatibility.Applies to lines 146, 322, and 357.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/server/src/terminalManager.test.ts`:
- Around line 354-362: The listen callback used with notFoundServer.listen in
terminalManager.test.ts incorrectly declares an error parameter; remove the
unused parameter from the callback and instead attach an 'error' event listener
on notFoundServer to reject the Promise on binding errors (or use once('error',
...) before calling listen), keeping the Promise resolution in the listen
callback (with no args). This updates the signature to match Node's
server.listen behavior and ensures binding errors are handled via the 'error'
event rather than a nonexistent callback argument.
🧹 Nitpick comments (3)
apps/web/src/components/Sidebar.tsx (2)
520-533: Consider extracting shared URL-opening logic to reduce duplication.The
onKeyDownhandler (lines 523-533) duplicates the external URL opening logic already present inopenWebPort. If the URL format or error handling changes, both locations must be updated.Extract the core action into a shared handler:
♻️ Suggested refactor
+ const handleOpenWebPort = useCallback( + (port: number) => { + if (!api) return; + void api.shell.openExternal(`http://localhost:${port}`).catch(() => undefined); + }, + [api], + ); + const openWebPort = useCallback( (event: MouseEvent<HTMLElement>, port: number) => { event.preventDefault(); event.stopPropagation(); - if (!api) return; - void api.shell.openExternal(`http://localhost:${port}`).catch(() => undefined); + handleOpenWebPort(port); }, - [api], + [api, handleOpenWebPort], );Then in the
onKeyDownhandler:onKeyDown={(event) => { if (event.key !== "Enter" && event.key !== " ") return; event.preventDefault(); event.stopPropagation(); - if (!api) return; - void api.shell - .openExternal( - `http://localhost:${terminalStatus.primaryWebPort!}`, - ) - .catch(() => undefined); + handleOpenWebPort(terminalStatus.primaryWebPort!); }}
514-538: Addcursor-pointerfor clickable element affordance.The span with
role="link"is interactive but lacks a pointer cursor, which may confuse users expecting visual feedback on hover.🖱️ Suggested fix
<span role="link" tabIndex={0} aria-label={terminalStatus.label} title={terminalStatus.label} - className={`inline-flex items-center justify-center ${terminalStatus.colorClass}`} + className={`inline-flex cursor-pointer items-center justify-center ${terminalStatus.colorClass}`}apps/server/src/terminalManager.ts (1)
1193-1198: Minor: redundant port normalization.
detectWebPortsalready returns a sorted array (line 1153:.toSorted(...)), so the outernormalizeRunningPortscall adds no value. Not a functional issue.♻️ Suggested simplification
const runningPorts = hasRunningSubprocess - ? normalizeRunningPorts( - await this.detectWebPorts(normalizeRunningPorts(activity.runningPorts)), - ) + ? await this.detectWebPorts(normalizeRunningPorts(activity.runningPorts)) : [];
Comment on lines
+354
to
+362
| await new Promise<void>((resolve, reject) => { | ||
| notFoundServer.listen(0, "127.0.0.1", (error?: Error) => { | ||
| if (error) { | ||
| reject(error); | ||
| return; | ||
| } | ||
| resolve(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Incorrect callback signature for server.listen().
The listen callback receives no arguments. Errors during binding are emitted via the 'error' event on the server, not passed to the callback. While this doesn't break the test (the parameter is simply unused), the signature is misleading.
🔧 Suggested fix
await new Promise<void>((resolve, reject) => {
- notFoundServer.listen(0, "127.0.0.1", (error?: Error) => {
- if (error) {
- reject(error);
- return;
- }
- resolve();
- });
+ notFoundServer.once("error", reject);
+ notFoundServer.listen(0, "127.0.0.1", () => {
+ notFoundServer.removeListener("error", reject);
+ resolve();
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await new Promise<void>((resolve, reject) => { | |
| notFoundServer.listen(0, "127.0.0.1", (error?: Error) => { | |
| if (error) { | |
| reject(error); | |
| return; | |
| } | |
| resolve(); | |
| }); | |
| }); | |
| await new Promise<void>((resolve, reject) => { | |
| notFoundServer.once("error", reject); | |
| notFoundServer.listen(0, "127.0.0.1", () => { | |
| notFoundServer.removeListener("error", reject); | |
| resolve(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@apps/server/src/terminalManager.test.ts` around lines 354 - 362, The listen
callback used with notFoundServer.listen in terminalManager.test.ts incorrectly
declares an error parameter; remove the unused parameter from the callback and
instead attach an 'error' event listener on notFoundServer to reject the Promise
on binding errors (or use once('error', ...) before calling listen), keeping the
Promise resolution in the listen callback (with no args). This updates the
signature to match Node's server.listen behavior and ensures binding errors are
handled via the 'error' event rather than a nonexistent callback argument.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/server/src/terminalProcessInspector/posix.ts`:
- Around line 67-71: The current check in posix.ts returns [] for any non-zero
lsof exit, preventing the ss fallback; change the logic so only result.code ===
1 is treated as "no results" and returns [], while any other non-zero
result.code should not return and should allow the code to fall through to the
ss fallback (or surface an error if you prefer). Locate the block referencing
result.code and lsof and update the conditional to return [] only when
result.code === 1, leaving other non-zero exit codes to proceed to the ss
handling.
In `@apps/server/src/terminalProcessInspector/utils.ts`:
- Around line 3-8: The call to Array.prototype.toSorted in normalizeRunningPorts
requires Node.js 20+, which may not be guaranteed; either declare "node": ">=20"
in package.json engines or replace toSorted usage in normalizeRunningPorts with
a runtime-compatible sort (e.g., copy array with slice() or spread and use
.sort((a,b)=>a-b)) so the function still returns a deduplicated, ascending array
and keeps the MAX_PORT_NUMBER check intact.
In `@apps/server/src/webPortInspector.ts`:
- Around line 66-75: When truncating the response in the res.on("data") handler
(when received >= WEB_PORT_PROBE_MAX_BODY_BYTES), call the existing settle(...)
logic with the partial body/chunks immediately before calling res.destroy() so
the promise is resolved with the truncated payload; ensure settle() is
idempotent (or guard it with a boolean like settled) so it is only invoked once
(both from the normal 'end' handler and this truncation path). In short: inside
the data handler for res.on("data") use the same settle(...) call that the 'end'
handler uses (passing the current chunks/received data and status), then destroy
the response to stop further data.
🧹 Nitpick comments (1)
apps/server/src/terminalManager.ts (1)
778-789: Consider bounding concurrent web‑port probes.
Promise.allover a large port list can spike outbound probes and latency. Batching keeps behavior more predictable under load.♻️ Suggested batching approach
- const checks = await Promise.all( - runningPorts.map(async (port) => ({ - port, - isWeb: await this.inspectWebPortCached(port), - })), - ); + const checks: Array<{ port: number; isWeb: boolean }> = []; + const batchSize = 8; + for (let index = 0; index < runningPorts.length; index += batchSize) { + const batch = runningPorts.slice(index, index + batchSize); + const results = await Promise.all( + batch.map(async (port) => ({ + port, + isWeb: await this.inspectWebPortCached(port), + })), + ); + checks.push(...results); + }As per coding guidelines: Maintain predictable behavior under load and during failures (session restarts, reconnects, partial streams).
Comment on lines
+67
to
+71
| if (result.code !== 0) { | ||
| // `lsof` returns 1 when there are no matching files/sockets. | ||
| // This is a valid "no results" outcome; avoid falling back to `ss`. | ||
| return []; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file structure and read the relevant lines
fd "posix.ts" -t f | head -5Repository: pingdotgg/codething-mvp
Length of output: 116
🏁 Script executed:
# Look at the full context of the file around lines 67-71
cat -n apps/server/src/terminalProcessInspector/posix.ts | sed -n '50,90p'Repository: pingdotgg/codething-mvp
Length of output: 1610
🌐 Web query:
lsof exit codes documentation meaning
💡 Result:
lsof’s exit status is primarily a “did my requested search succeed?” indicator (not just “did I print any lines?”):
- 0: No errors detected, and
lsofwas able to list information for all specified search arguments (and if you didn’t specify any files to search for, success is also 0). [2] [3] - 1: Some error was detected including “search failure” cases such as failing to locate a requested name/address/PID/UID/file/etc., or not finding all specified files/filesystems. This often happens when your filters match nothing for one of the specified search items. [2] [3]
- Repeat mode special case (
-rwith+): when repeat mode ends because “no open files are listed”, exit is 0 if any open files were ever listed in earlier cycles, otherwise 1. [1]
Related option:
-Q: makes “failed to find a search item” still exit 0, but it does not suppress exit 1 for generic/non-search execution errors. [2] [6]
Fall back to ss on lsof failures other than "no results".
Currently, any non-zero lsof exit returns [] and skips the ss fallback. The comment states that exit code 1 means "no matching files/sockets", but lsof exit code 1 actually indicates broader search failures—it can mean no matches, failed to locate requested items, or incomplete results. Other exit codes indicate errors like lsof being unavailable. Treat exit code 1 as a "no results" outcome and fall through to ss for other errors.
🛠️ Suggested fix
if (result.code !== 0) {
// `lsof` returns 1 when there are no matching files/sockets.
+ // Other codes indicate errors; fall back to ss.
- // This is a valid "no results" outcome; avoid falling back to `ss`.
- return [];
+ if (result.code === 1) return [];
+ break;
}(Alternatively, throw an error to surface lsof failures explicitly.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (result.code !== 0) { | |
| // `lsof` returns 1 when there are no matching files/sockets. | |
| // This is a valid "no results" outcome; avoid falling back to `ss`. | |
| return []; | |
| } | |
| if (result.code !== 0) { | |
| // `lsof` returns 1 when there are no matching files/sockets. | |
| // Other codes indicate errors; fall back to ss. | |
| if (result.code === 1) return []; | |
| break; | |
| } |
🤖 Prompt for AI Agents
In `@apps/server/src/terminalProcessInspector/posix.ts` around lines 67 - 71, The
current check in posix.ts returns [] for any non-zero lsof exit, preventing the
ss fallback; change the logic so only result.code === 1 is treated as "no
results" and returns [], while any other non-zero result.code should not return
and should allow the code to fall through to the ss fallback (or surface an
error if you prefer). Locate the block referencing result.code and lsof and
update the conditional to return [] only when result.code === 1, leaving other
non-zero exit codes to proceed to the ss handling.
- add `runningPorts` to terminal activity contract and validation - detect/listen ports in server subprocess inspection and emit activity updates on port changes - persist terminal-to-port state in web store and show active ports in sidebar indicator - update server/web/contracts tests for port-aware activity behavior
- Add `-a` to the `lsof` command in terminal port detection - Prevent mismatched results by combining TCP listen and PID filters correctly
- Probe terminal subprocess ports and keep only likely web servers - Cache/dedupe port probes and ignore root-path 404 responses - Show a globe status icon in Sidebar that opens `http://localhost:<port>`
juliusmarminge
force-pushed
the
codething/50e6e9da
branch
from
259251c to
b3e967e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/server/src/terminalManager.ts`:
- Around line 796-800: Replace the double-normalization around detectWebPorts:
remove the outer normalizeRunningPorts(...) call and instead call detectWebPorts
with the normalized activity.runningPorts directly by invoking
this.detectWebPorts(normalizeRunningPorts(activity.runningPorts)); update the
call site where normalizeRunningPorts(await this.detectWebPorts(...)) is used so
only normalizeRunningPorts(activity.runningPorts) is passed into detectWebPorts
(referencing the functions normalizeRunningPorts and detectWebPorts and the
variable activity.runningPorts).
🧹 Nitpick comments (4)
apps/server/src/terminalManager.ts (1)
839-844: Consider removing redundantnormalizeRunningPortscall.The outer
normalizeRunningPorts()wrapping the result ofdetectWebPorts()is redundant sincedetectWebPorts()already returns a sorted, deduplicated array via.toSorted(). This doesn't cause bugs but adds unnecessary overhead.♻️ Suggested simplification
const runningPorts = hasRunningSubprocess - ? normalizeRunningPorts( - await this.detectWebPorts(normalizeRunningPorts(activity.runningPorts)), - ) + ? await this.detectWebPorts(normalizeRunningPorts(activity.runningPorts)) : [];apps/web/src/components/Sidebar.tsx (3)
113-119: Consider extracting shared port normalization logic.This port aggregation and normalization logic duplicates
normalizeRunningPortsfromstore.ts. While the duplication is minor, consider importing or extracting a shared utility to ensure consistent behavior.
561-600: Minor: Refactor duplicated port-opening logic inonKeyDownhandler.The
onKeyDownhandler (Lines 572-581) duplicates the port-opening logic instead of reusingopenWebPort. This creates maintenance overhead if the behavior needs to change.♻️ Proposed refactor to reuse openWebPort
onKeyDown={(event) => { if (event.key !== "Enter" && event.key !== " ") return; - event.preventDefault(); - event.stopPropagation(); - if (!api) return; - void api.shell - .openExternal( - `http://localhost:${terminalStatus.primaryWebPort!}`, - ) - .catch(() => undefined); + openWebPort( + event as unknown as MouseEvent<HTMLElement>, + terminalStatus.primaryWebPort!, + ); }}Alternatively, extract a shared handler that accepts both event types:
const handleOpenWebPort = useCallback( (event: { preventDefault: () => void; stopPropagation: () => void }, port: number) => { event.preventDefault(); event.stopPropagation(); if (!api) return; void api.shell.openExternal(`http://localhost:${port}`).catch(() => undefined); }, [api], );
563-587: Consider usingrole="button"instead ofrole="link"for the interactive port indicator.The element behaves like a button (triggers an action) rather than a link (navigates to a URL). Using
role="button"would be more semantically accurate for accessibility tools.♻️ Proposed fix
<span - role="link" + role="button" tabIndex={0}
- Pass running terminal IDs/ports from `ChatView` into `ThreadTerminalDrawer` - Add runtime badges/icons for running terminals and localhost web ports - Make sidebar thread rows keyboard-activatable and add web-port quick open button
Member
Author
|
@greptileai review |
- Include terminal root PID in port scans so listener-only sessions are marked active - Improve web port detection for slow/large HTML responses and redirects - Add integration-style tests for terminal subprocess and web port inspectors
Co-authored-by: codex <codex@users.noreply.github.com>
github-actions
Bot
added
the
vouch:trusted
DavidIlie
added a commit
to DavidIlie/t3code
that referenced
this pull request
Mar 13, 2026
…icker Port upstream commits 3d35868..f70cb5d: - Add pull-request thread setup for local and worktree modes (pingdotgg#42) - Refine branch picker behavior and gate /checkout-pr by thread type (pingdotgg#43) Includes PR resolution via GitHub CLI, worktree-aware PR checkout, shared branch picker filtering, deferred query, conditional virtualization, and /checkout-pr gating to local draft threads only.
Co-authored-by: codex <codex@users.noreply.github.com>
github-actions
Bot
added
the
size:XL
Contributor
ApprovabilityVerdict: Needs human review This PR introduces a substantial new feature for detecting and displaying terminal listening ports, including spawning external processes (lsof/ss/powershell), HTTP probing of localhost ports, caching logic, and UI updates with click-to-open functionality. Multiple unresolved review comments identify potential bugs including wrong icons and lsof fallback issues. The scope and complexity warrant human review. You can customize Macroscope's approvability policy. Learn more. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Windows misses grandchild ports unlike POSIX full-tree traversal
- Replaced
collectWindowsChildPids(direct-children-only) withcollectWindowsProcessFamilyPidsthat fetches all process PIDs with parent info and performs BFS traversal matching the POSIX implementation.
- Replaced
- ✅ Fixed: Exported
subprocessCheckerToInspectoris never imported or used- Removed the unused
subprocessCheckerToInspectorfunction and its unused local type imports fromindex.ts.
- Removed the unused
- ✅ Fixed: Triplicated
normalizeRunningPortsacross web app files- Exported the existing
normalizeRunningPortsfromterminalStateStore.tsand replaced the duplicate definitions inSidebar.tsxandThreadTerminalDrawer.tsxwith imports.
- Exported the existing
Or push these changes by commenting:
@cursor push 755bf3aa0d
Preview (755bf3aa0d)
diff --git a/apps/server/src/terminalProcessInspector/index.ts b/apps/server/src/terminalProcessInspector/index.ts
--- a/apps/server/src/terminalProcessInspector/index.ts
+++ b/apps/server/src/terminalProcessInspector/index.ts
@@ -1,11 +1,6 @@
import { collectPosixProcessFamilyPids, checkPosixListeningPorts } from "./posix";
-import {
- type TerminalSubprocessActivity,
- type TerminalSubprocessChecker,
- type TerminalSubprocessInspector,
- type TerminalWebPortInspector,
-} from "./types";
-import { checkWindowsListeningPorts, collectWindowsChildPids } from "./win32";
+import { type TerminalSubprocessActivity } from "./types";
+import { checkWindowsListeningPorts, collectWindowsProcessFamilyPids } from "./win32";
export { arePortListsEqual, normalizeRunningPorts } from "./utils";
@@ -24,11 +19,15 @@
}
if (process.platform === "win32") {
- const childPids = await collectWindowsChildPids(terminalPid);
- const processPidsForPortScan = [terminalPid, ...childPids];
- const runningPorts = await checkWindowsListeningPorts(processPidsForPortScan);
+ const processFamilyPids = await collectWindowsProcessFamilyPids(terminalPid);
+ if (processFamilyPids.length === 0) {
+ return { hasRunningSubprocess: false, runningPorts: [] };
+ }
+
+ const subprocessPids = processFamilyPids.filter((pid) => pid !== terminalPid);
+ const runningPorts = await checkWindowsListeningPorts(processFamilyPids);
return {
- hasRunningSubprocess: childPids.length > 0 || runningPorts.length > 0,
+ hasRunningSubprocess: subprocessPids.length > 0 || runningPorts.length > 0,
runningPorts,
};
}
@@ -45,12 +44,3 @@
runningPorts,
};
}
-
-export function subprocessCheckerToInspector(
- subprocessChecker: TerminalSubprocessChecker,
-): TerminalSubprocessInspector {
- return async (terminalPid: number) => ({
- hasRunningSubprocess: await subprocessChecker(terminalPid),
- runningPorts: [],
- });
-}
diff --git a/apps/server/src/terminalProcessInspector/win32.ts b/apps/server/src/terminalProcessInspector/win32.ts
--- a/apps/server/src/terminalProcessInspector/win32.ts
+++ b/apps/server/src/terminalProcessInspector/win32.ts
@@ -1,27 +1,56 @@
import { runProcess } from "../processRunner";
-import { parsePidList, parsePortList } from "./utils";
+import { parsePortList } from "./utils";
-export async function collectWindowsChildPids(terminalPid: number): Promise<number[]> {
+export async function collectWindowsProcessFamilyPids(terminalPid: number): Promise<number[]> {
const command = [
- `$children = Get-CimInstance Win32_Process -Filter "ParentProcessId = ${terminalPid}" -ErrorAction SilentlyContinue`,
- "if (-not $children) { exit 0 }",
- "$children | Select-Object -ExpandProperty ProcessId",
+ "$procs = Get-CimInstance Win32_Process -Property ProcessId,ParentProcessId -ErrorAction SilentlyContinue",
+ "if (-not $procs) { exit 0 }",
+ '$procs | ForEach-Object { "$($_.ProcessId) $($_.ParentProcessId)" }',
].join("; ");
try {
const result = await runProcess(
"powershell.exe",
["-NoProfile", "-NonInteractive", "-Command", command],
{
- timeoutMs: 1_500,
+ timeoutMs: 2_000,
allowNonZeroExit: true,
- maxBufferBytes: 32_768,
+ maxBufferBytes: 262_144,
outputMode: "truncate",
},
);
if (result.code !== 0) {
return [];
}
- return parsePidList(result.stdout);
+
+ const childrenByParentPid = new Map<number, number[]>();
+ for (const line of result.stdout.split(/\r?\n/g)) {
+ const [pidRaw, ppidRaw] = line.trim().split(/\s+/g);
+ const pid = Number(pidRaw);
+ const ppid = Number(ppidRaw);
+ if (!Number.isInteger(pid) || !Number.isInteger(ppid)) continue;
+ const children = childrenByParentPid.get(ppid);
+ if (children) {
+ children.push(pid);
+ } else {
+ childrenByParentPid.set(ppid, [pid]);
+ }
+ }
+
+ const processFamily = new Set<number>([terminalPid]);
+ const pendingParents = [terminalPid];
+ while (pendingParents.length > 0) {
+ const parentPid = pendingParents.shift();
+ if (!parentPid) continue;
+ const childPids = childrenByParentPid.get(parentPid);
+ if (!childPids || childPids.length === 0) continue;
+ for (const childPid of childPids) {
+ if (processFamily.has(childPid)) continue;
+ processFamily.add(childPid);
+ pendingParents.push(childPid);
+ }
+ }
+
+ return [...processFamily];
} catch {
return [];
}
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -61,7 +61,11 @@
import { isTerminalFocused } from "../lib/terminalFocus";
import { isLinuxPlatform, isMacPlatform, newCommandId, newProjectId } from "../lib/utils";
import { useStore } from "../store";
-import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore";
+import {
+ normalizeRunningPorts,
+ selectThreadTerminalState,
+ useTerminalStateStore,
+} from "../terminalStateStore";
import { useUiStateStore } from "../uiStateStore";
import {
resolveShortcutCommand,
@@ -204,13 +208,6 @@
);
}
-function normalizeRunningPorts(rawPorts: number[] | undefined): number[] {
- if (!rawPorts) return [];
- return [...new Set(rawPorts)]
- .filter((port) => Number.isInteger(port) && port > 0 && port <= 65_535)
- .toSorted((left, right) => left - right);
-}
-
function terminalStatusFromTerminalState(
runningTerminalIds: string[],
runningTerminalPorts: Record<string, number[]>,
diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx
--- a/apps/web/src/components/ThreadTerminalDrawer.tsx
+++ b/apps/web/src/components/ThreadTerminalDrawer.tsx
@@ -33,7 +33,11 @@
type ThreadTerminalGroup,
} from "../types";
import { readNativeApi } from "~/nativeApi";
-import { selectTerminalEventEntries, useTerminalStateStore } from "../terminalStateStore";
+import {
+ normalizeRunningPorts,
+ selectTerminalEventEntries,
+ useTerminalStateStore,
+} from "../terminalStateStore";
const MIN_DRAWER_HEIGHT = 180;
const MAX_DRAWER_HEIGHT_RATIO = 0.75;
@@ -81,13 +85,6 @@
extraWebPortCount: number;
}
-function normalizeRunningPorts(rawPorts: number[] | undefined): number[] {
- if (!rawPorts) return [];
- return [...new Set(rawPorts)]
- .filter((port) => Number.isInteger(port) && port > 0 && port <= 65_535)
- .toSorted((left, right) => left - right);
-}
-
function terminalRuntimeStatus(
terminalId: string,
runningTerminalIds: Set<string>,
diff --git a/apps/web/src/terminalStateStore.ts b/apps/web/src/terminalStateStore.ts
--- a/apps/web/src/terminalStateStore.ts
+++ b/apps/web/src/terminalStateStore.ts
@@ -62,7 +62,7 @@
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));
}
-function normalizeRunningPorts(ports: readonly number[] | undefined): number[] {
+export function normalizeRunningPorts(ports: readonly number[] | undefined): number[] {
if (!ports || ports.length === 0) return [];
return [...new Set(ports)]
.filter((port) => Number.isInteger(port) && port > 0 && port <= 65_535)You can send follow-ups to the cloud agent here.
- Add Effect layers for process and port probing - Wire inspectors into server startup and tests - Co-authored-by: codex <codex@users.noreply.github.com>
github-actions
Bot
added
size:XXL
- Rename terminalProcessInspector modules to process - Fold web port inspector exports into the new package - Update server and terminal manager imports
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Web port probe cache grows without bound
- Added a cleanup pass in inspectWebPortCached that deletes all expired entries from the map whenever a cache miss occurs, preventing unbounded growth.
Or push these changes by commenting:
@cursor push 2d3f25de8a
Preview (2d3f25de8a)
diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts
--- a/apps/server/src/terminal/Layers/Manager.ts
+++ b/apps/server/src/terminal/Layers/Manager.ts
@@ -613,6 +613,12 @@
return cached.isWeb;
}
+ for (const [p, entry] of webPortProbeCache) {
+ if (now - entry.checkedAt > webPortProbeCacheTtlMs) {
+ webPortProbeCache.delete(p);
+ }
+ }
+
const isWeb = yield* webPortInspector(port).pipe(Effect.catch(() => Effect.succeed(false)));
webPortProbeCache.set(port, {
isWeb,You can send follow-ups to the cloud agent here.
- add layer-level coverage for slow, large, and 404 responses - move inspector tests beside the layer and wire FetchHttpClient in terminal tests
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Timeout errors double-wrapped by subsequent mapError
- Added a _tag check in mapError to pass through errors that are already WebPortInspectionError instances, preserving the original timeout detail message.
- ✅ Fixed:
extraWebPortCountcomputed but never used in rendering- Removed the unused extraWebPortCount field from the TerminalRuntimeStatus interface and its computation in terminalRuntimeStatus().
Or push these changes by commenting:
@cursor push e11ef1b119
Preview (e11ef1b119)
diff --git a/apps/server/src/process/Layers/WebPortInspector.ts b/apps/server/src/process/Layers/WebPortInspector.ts
--- a/apps/server/src/process/Layers/WebPortInspector.ts
+++ b/apps/server/src/process/Layers/WebPortInspector.ts
@@ -127,15 +127,22 @@
onSome: Effect.succeed,
}),
),
- Effect.mapError(
- (cause) =>
- new WebPortInspectionError({
- port,
- host,
- detail: "Failed to execute HTTP probe request.",
- cause,
- }),
- ),
+ Effect.mapError((cause) => {
+ if (
+ typeof cause === "object" &&
+ cause !== null &&
+ "_tag" in cause &&
+ cause._tag === "WebPortInspectionError"
+ ) {
+ return cause as WebPortInspectionError;
+ }
+ return new WebPortInspectionError({
+ port,
+ host,
+ detail: "Failed to execute HTTP probe request.",
+ cause,
+ });
+ }),
);
});
diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx
--- a/apps/web/src/components/ThreadTerminalDrawer.tsx
+++ b/apps/web/src/components/ThreadTerminalDrawer.tsx
@@ -78,7 +78,6 @@
interface TerminalRuntimeStatus {
label: string;
primaryWebPort: number | null;
- extraWebPortCount: number;
}
function normalizeRunningPorts(rawPorts: number[] | undefined): number[] {
@@ -99,7 +98,6 @@
const runningPorts = normalizeRunningPorts(runningTerminalPorts[terminalId]);
const primaryWebPort = runningPorts[0] ?? null;
- const extraWebPortCount = runningPorts.length > 1 ? runningPorts.length - 1 : 0;
const label =
runningPorts.length === 0
? "Terminal process running"
@@ -110,7 +108,6 @@
return {
label,
primaryWebPort,
- extraWebPortCount,
};
}You can send follow-ups to the cloud agent here.
- Co-locate inspection errors and types with the service modules - Remove legacy subprocessChecker plumbing from the terminal manager
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Windows subprocess inspection misses grandchild processes unlike POSIX
- Changed collectWindowsChildPids to fetch the full process table (ProcessId + ParentProcessId) and perform a BFS traversal from the terminal PID, matching the POSIX implementation's behavior for discovering all descendant processes.
Or push these changes by commenting:
@cursor push 89b175a1cb
Preview (89b175a1cb)
diff --git a/apps/server/src/process/win32.ts b/apps/server/src/process/win32.ts
--- a/apps/server/src/process/win32.ts
+++ b/apps/server/src/process/win32.ts
@@ -1,7 +1,7 @@
import { Effect } from "effect";
import type { TerminalProcessInspectionError } from "./Services/TerminalProcessInspector";
-import { parsePidList, parsePortList } from "./utils";
+import { parsePortList } from "./utils";
interface InspectorCommandResult {
readonly stdout: string;
@@ -27,9 +27,9 @@
runCommand: WindowsRunCommand,
): Effect.fn.Return<number[], TerminalProcessInspectionError> {
const command = [
- `$children = Get-CimInstance Win32_Process -Filter "ParentProcessId = ${terminalPid}" -ErrorAction SilentlyContinue`,
- "if (-not $children) { exit 0 }",
- "$children | Select-Object -ExpandProperty ProcessId",
+ "$procs = Get-CimInstance Win32_Process -Property ProcessId,ParentProcessId -ErrorAction SilentlyContinue",
+ "if (-not $procs) { exit 0 }",
+ '$procs | ForEach-Object { "$($_.ProcessId) $($_.ParentProcessId)" }',
].join("; ");
const result = yield* runCommand({
operation: "TerminalProcessInspector.collectWindowsChildPids",
@@ -37,12 +37,40 @@
command: "powershell.exe",
args: ["-NoProfile", "-NonInteractive", "-Command", command],
timeoutMs: 1_500,
- maxOutputBytes: 32_768,
+ maxOutputBytes: 262_144,
});
if (result.exitCode !== 0) {
return [];
}
- return parsePidList(result.stdout);
+
+ const childrenByParentPid = new Map<number, number[]>();
+ for (const line of result.stdout.split(/\r?\n/g)) {
+ const [pidRaw, ppidRaw] = line.trim().split(/\s+/g);
+ const pid = Number(pidRaw);
+ const ppid = Number(ppidRaw);
+ if (!Number.isInteger(pid) || !Number.isInteger(ppid)) continue;
+ const children = childrenByParentPid.get(ppid);
+ if (children) {
+ children.push(pid);
+ } else {
+ childrenByParentPid.set(ppid, [pid]);
+ }
+ }
+
+ const family = new Set<number>();
+ const pending = [terminalPid];
+ while (pending.length > 0) {
+ const parentPid = pending.shift()!;
+ const childPids = childrenByParentPid.get(parentPid);
+ if (!childPids || childPids.length === 0) continue;
+ for (const childPid of childPids) {
+ if (family.has(childPid)) continue;
+ family.add(childPid);
+ pending.push(childPid);
+ }
+ }
+
+ return [...family];
});
export const checkWindowsListeningPorts = Effect.fn("process.checkWindowsListeningPorts")(You can send follow-ups to the cloud agent here.
- Share port normalization across terminal selectors - Preserve typed web probe errors and walk Windows child trees - Co-authored-by: codex <codex@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Duplicated
normalizeRunningPortsacross server and web packages- Extracted normalizeRunningPorts into @t3tools/shared/port and updated both server and web packages to import from the shared module.
- ✅ Fixed: Duplicated port label construction in sidebar and drawer
- Extracted portStatusLabel helper into @t3tools/shared/port and updated both ThreadTerminalDrawer.tsx and Sidebar.tsx to use it.
- ✅ Fixed: Negative web port probe results are cached for TTL
- Modified inspectWebPortCached to only cache successful probe results, so transient failures are re-probed on the next poll cycle.
Or push these changes by commenting:
@cursor push b1ad5fac52
Preview (b1ad5fac52)
diff --git a/apps/server/src/process/utils.ts b/apps/server/src/process/utils.ts
--- a/apps/server/src/process/utils.ts
+++ b/apps/server/src/process/utils.ts
@@ -1,11 +1,6 @@
-export const MAX_PORT_NUMBER = 65_535;
+import { MAX_PORT_NUMBER, normalizeRunningPorts } from "@t3tools/shared/port";
-export function normalizeRunningPorts(ports: number[]): number[] {
- if (ports.length === 0) return [];
- return [...new Set(ports)]
- .filter((port) => Number.isInteger(port) && port > 0 && port <= MAX_PORT_NUMBER)
- .toSorted((left, right) => left - right);
-}
+export { MAX_PORT_NUMBER, normalizeRunningPorts };
export function parsePidList(stdout: string): number[] {
const pids: number[] = [];
diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts
--- a/apps/server/src/terminal/Layers/Manager.ts
+++ b/apps/server/src/terminal/Layers/Manager.ts
@@ -613,12 +613,17 @@
return cached.isWeb;
}
- const isWeb = yield* webPortInspector(port).pipe(Effect.catch(() => Effect.succeed(false)));
- webPortProbeCache.set(port, {
- isWeb,
- checkedAt: Date.now(),
- });
- return isWeb;
+ const isWeb = yield* webPortInspector(port).pipe(
+ Effect.map((result) => ({ result, probeSucceeded: true }) as const),
+ Effect.catch(() => Effect.succeed({ result: false, probeSucceeded: false } as const)),
+ );
+ if (isWeb.probeSucceeded) {
+ webPortProbeCache.set(port, {
+ isWeb: isWeb.result,
+ checkedAt: Date.now(),
+ });
+ }
+ return isWeb.result;
});
const detectWebPorts = (runningPorts: number[]) =>
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -61,11 +61,8 @@
import { isTerminalFocused } from "../lib/terminalFocus";
import { isLinuxPlatform, isMacPlatform, newCommandId, newProjectId } from "../lib/utils";
import { useStore } from "../store";
-import {
- normalizeRunningPorts,
- selectThreadTerminalState,
- useTerminalStateStore,
-} from "../terminalStateStore";
+import { normalizeRunningPorts, portStatusLabel } from "@t3tools/shared/port";
+import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore";
import { useUiStateStore } from "../uiStateStore";
import {
resolveShortcutCommand,
@@ -219,15 +216,10 @@
const runningPorts = normalizeRunningPorts(
runningTerminalIds.flatMap((terminalId) => runningTerminalPorts[terminalId] ?? []),
);
- const primaryWebPort = runningPorts[0] ?? null;
+ const { label, primaryWebPort } = portStatusLabel(runningPorts);
return {
- label:
- primaryWebPort === null
- ? "Terminal process running"
- : runningPorts.length === 1
- ? `Open web server: http://localhost:${primaryWebPort}`
- : `Open web server: http://localhost:${primaryWebPort} (detected web ports: ${runningPorts.join(", ")})`,
+ label,
colorClass:
primaryWebPort === null
? "text-teal-600 dark:text-teal-300/90"
diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx
--- a/apps/web/src/components/ThreadTerminalDrawer.tsx
+++ b/apps/web/src/components/ThreadTerminalDrawer.tsx
@@ -33,11 +33,8 @@
type ThreadTerminalGroup,
} from "../types";
import { readNativeApi } from "~/nativeApi";
-import {
- normalizeRunningPorts,
- selectTerminalEventEntries,
- useTerminalStateStore,
-} from "../terminalStateStore";
+import { normalizeRunningPorts, portStatusLabel, type PortStatusLabel } from "@t3tools/shared/port";
+import { selectTerminalEventEntries, useTerminalStateStore } from "../terminalStateStore";
const MIN_DRAWER_HEIGHT = 180;
const MAX_DRAWER_HEIGHT_RATIO = 0.75;
@@ -79,33 +76,17 @@
return entries.filter((entry) => entry.id > lastAppliedTerminalEventId);
}
-interface TerminalRuntimeStatus {
- label: string;
- primaryWebPort: number | null;
-}
-
function terminalRuntimeStatus(
terminalId: string,
runningTerminalIds: Set<string>,
runningTerminalPorts: Record<string, number[]>,
-): TerminalRuntimeStatus | null {
+): PortStatusLabel | null {
if (!runningTerminalIds.has(terminalId)) {
return null;
}
const runningPorts = normalizeRunningPorts(runningTerminalPorts[terminalId]);
- const primaryWebPort = runningPorts[0] ?? null;
- const label =
- runningPorts.length === 0
- ? "Terminal process running"
- : runningPorts.length === 1
- ? `Open web server: http://localhost:${primaryWebPort}`
- : `Open web server: http://localhost:${primaryWebPort} (detected web ports: ${runningPorts.join(", ")})`;
-
- return {
- label,
- primaryWebPort,
- };
+ return portStatusLabel(runningPorts);
}
function terminalThemeFromApp(): ITheme {
diff --git a/apps/web/src/terminalStateStore.ts b/apps/web/src/terminalStateStore.ts
--- a/apps/web/src/terminalStateStore.ts
+++ b/apps/web/src/terminalStateStore.ts
@@ -6,6 +6,8 @@
*/
import { ThreadId, type TerminalEvent } from "@t3tools/contracts";
+import { normalizeRunningPorts } from "@t3tools/shared/port";
+export { normalizeRunningPorts } from "@t3tools/shared/port";
import { create } from "zustand";
import { createJSONStorage, persist } from "zustand/middleware";
import { resolveStorage } from "./lib/storage";
@@ -62,13 +64,6 @@
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));
}
-export function normalizeRunningPorts(ports: readonly number[] | undefined): number[] {
- if (!ports || ports.length === 0) return [];
- return [...new Set(ports)]
- .filter((port) => Number.isInteger(port) && port > 0 && port <= 65_535)
- .toSorted((left, right) => left - right);
-}
-
function normalizeRunningTerminalPorts(
runningTerminalPorts: Record<string, number[]> | undefined,
terminalIds: string[],
diff --git a/packages/shared/package.json b/packages/shared/package.json
--- a/packages/shared/package.json
+++ b/packages/shared/package.json
@@ -51,6 +51,10 @@
"./projectScripts": {
"types": "./src/projectScripts.ts",
"import": "./src/projectScripts.ts"
+ },
+ "./port": {
+ "types": "./src/port.ts",
+ "import": "./src/port.ts"
}
},
"scripts": {
diff --git a/packages/shared/src/port.ts b/packages/shared/src/port.ts
new file mode 100644
--- /dev/null
+++ b/packages/shared/src/port.ts
@@ -1,0 +1,24 @@
+export const MAX_PORT_NUMBER = 65_535;
+
+export function normalizeRunningPorts(ports: readonly number[] | undefined): number[] {
+ if (!ports || ports.length === 0) return [];
+ return [...new Set(ports)]
+ .filter((port) => Number.isInteger(port) && port > 0 && port <= MAX_PORT_NUMBER)
+ .toSorted((left, right) => left - right);
+}
+
+export interface PortStatusLabel {
+ label: string;
+ primaryWebPort: number | null;
+}
+
+export function portStatusLabel(runningPorts: readonly number[]): PortStatusLabel {
+ const primaryWebPort = runningPorts[0] ?? null;
+ const label =
+ runningPorts.length === 0
+ ? "Terminal process running"
+ : runningPorts.length === 1
+ ? `Open web server: http://localhost:${primaryWebPort}`
+ : `Open web server: http://localhost:${primaryWebPort} (detected web ports: ${runningPorts.join(", ")})`;
+ return { label, primaryWebPort };
+}You can send follow-ups to the cloud agent here.
- Fallback from `lsof` to `ss` on Linux and keep confirmed web ports cached - Include command and cause details in subprocess errors, with throttled repeat warnings - Add hover tooltips to terminal status icons in the sidebar and drawer
Co-authored-by: codex <codex@users.noreply.github.com>
- Reformat terminal process inspection branching - Simplify web port probing and error handling
| ))} | ||
| {normalizedTerminalIds.length > 1 && ( | ||
| <Popover> | ||
| <PopoverTrigger |
Contributor
There was a problem hiding this comment.
🟢 Low components/ThreadTerminalDrawer.tsx:1394
The close terminal button's XIcon is placed outside the render prop's button element, inside PopoverTrigger instead. This renders an empty button with no visible icon. Move <XIcon className="size-2.5" /> inside the button element in the render prop, following the pattern used for the Globe and TerminalSquare icons on lines 1369-1378 and 1344-1351.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ThreadTerminalDrawer.tsx around line 1394:
The close terminal button's `XIcon` is placed outside the `render` prop's button element, inside `PopoverTrigger` instead. This renders an empty button with no visible icon. Move `<XIcon className="size-2.5" />` inside the button element in the `render` prop, following the pattern used for the Globe and TerminalSquare icons on lines 1369-1378 and 1344-1351.
Evidence trail:
apps/web/src/components/ThreadTerminalDrawer.tsx lines 1394-1407 (REVIEWED_COMMIT): XIcon is a child of PopoverTrigger, but the button in the render prop is self-closing with no children.
Compare with lines 1369-1378 (Globe pattern): Globe icon is inside the button element in the render prop.
Compare with lines 1344-1351 (TerminalSquare pattern): TerminalSquare icon is inside the span element in the render prop.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Sidebar web port button shows wrong icon
- Imported
Globefrom lucide-react and replacedTerminalIconwithGlobein the web-port button branch of the sidebar.
- Imported
- ✅ Fixed: Unreachable fallback in web port error propagation
- Removed the always-true
ipv6Result._tag === "Failure"branch and its dead code fallback, now directly propagating the IPv4 error which is tried first and more commonly relevant.
- Removed the always-true
Or push these changes by commenting:
@cursor push 5d70913a22
Preview (5d70913a22)
diff --git a/apps/server/src/process/Layers/WebPortInspector.ts b/apps/server/src/process/Layers/WebPortInspector.ts
--- a/apps/server/src/process/Layers/WebPortInspector.ts
+++ b/apps/server/src/process/Layers/WebPortInspector.ts
@@ -164,10 +164,6 @@
return false;
}
- if (ipv6Result._tag === "Failure") {
- return yield* Effect.failCause(ipv6Result.cause);
- }
-
return yield* Effect.failCause(ipv4Result.cause);
});
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -8,6 +8,7 @@
SearchIcon,
SettingsIcon,
SquarePenIcon,
+ Globe,
TerminalIcon,
TriangleAlertIcon,
} from "lucide-react";
@@ -674,7 +675,7 @@
className={`inline-flex cursor-pointer items-center justify-center rounded-sm ${terminalStatus.colorClass} outline-hidden transition-colors hover:bg-accent hover:text-sky-700 focus-visible:ring-1 focus-visible:ring-ring dark:hover:text-sky-200`}
onClick={handleTerminalStatusClick}
>
- <TerminalIcon className={`size-3 ${terminalStatus.pulse ? "animate-pulse" : ""}`} />
+ <Globe className={`size-3 ${terminalStatus.pulse ? "animate-pulse" : ""}`} />
</button>
))}
<div className="flex min-w-12 justify-end">You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit b0bc101. Configure here.
| className={`inline-flex cursor-pointer items-center justify-center rounded-sm ${terminalStatus.colorClass} outline-hidden transition-colors hover:bg-accent hover:text-sky-700 focus-visible:ring-1 focus-visible:ring-ring dark:hover:text-sky-200`} | ||
| onClick={handleTerminalStatusClick} | ||
| > | ||
| <TerminalIcon className={`size-3 ${terminalStatus.pulse ? "animate-pulse" : ""}`} /> |
Contributor
There was a problem hiding this comment.
Sidebar web port button shows wrong icon
Medium Severity
The sidebar's web-port button renders TerminalIcon instead of Globe. The ThreadTerminalDrawer correctly imports and uses Globe for the web-port case, and the sidebar already has all the conditional rendering infrastructure (different color class, clickable button, handleTerminalStatusClick), but the icon inside the button branch was never changed. Globe is not imported in Sidebar.tsx at all.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b0bc101. Configure here.
| return yield* Effect.failCause(ipv6Result.cause); | ||
| } | ||
|
|
||
| return yield* Effect.failCause(ipv4Result.cause); |
Contributor
There was a problem hiding this comment.
Unreachable fallback in web port error propagation
Low Severity
After the check at line 163, both ipv4Result and ipv6Result must be failures, so ipv6Result._tag === "Failure" at line 167 is always true and the final return yield* Effect.failCause(ipv4Result.cause) at line 171 is dead code. This also means the IPv6 error is always propagated even though IPv4 was tried first, which is unintuitive.
Reviewed by Cursor Bugbot for commit b0bc101. Configure here.
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
runningPortsalongsidehasRunningSubprocesslsof/ss) and Windows (PowerShell CIM/NetTCP)runningTerminalPorts) and clear it on terminal restart/exit/error/closeTesting
apps/server/src/terminalManager.test.ts: verifies activity events emit on subprocess state and port changes, including port normalization and clearing when inactiveapps/web/src/store.test.ts: verifies reducer tracksrunningTerminalPorts, updates on activity changes, and removes port state when terminals stop/existpackages/contracts/src/terminal.test.ts: verifiesterminalActivityEventSchemaacceptsrunningPortsapps/web/src/persistenceSchema.test.tsapps/web/src/worktreeCleanup.test.tsSummary by CodeRabbit
New Features
Public Events
Behavior
Persistence
Tests
Note
Medium Risk
Adds periodic OS-level process/port inspection (
ps/lsof/ss/PowerShell) and HTTP probing to terminal polling, which can impact runtime behavior and introduces platform-specific edge cases despite bounded timeouts and error throttling.Overview
Terminal
activityevents now carry normalizedrunningPortsalongsidehasRunningSubprocess, with contracts updated to validate port ranges.On the server, a new
TerminalProcessInspectorLivereplaces the old boolean subprocess check by collecting process families and scanning listening ports on POSIX (ps+lsof, fallback toss) and Windows (PowerShell CIM +Get-NetTCPConnection); a newWebPortInspectorLiveprobes localhost over IPv4/IPv6 to decide which ports are likely web servers (404 ignored), with TTL caching and transient-failure tolerance.TerminalManageremits activity events when either the subprocess flag or the detected web-port set changes, and throttles repeated inspector error logs.On the web side, thread terminal state now persists
runningTerminalPortsper terminal, clears it on start/restart/exit/error/close, and the sidebar + terminal drawer show a clickable web indicator that openshttp://localhost:<port>when a primary port is detected.Reviewed by Cursor Bugbot for commit b0bc101. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Track listening ports in terminal activity events and display detected web ports in the UI
TerminalProcessInspectorLive) that discovers child PIDs and listening TCP ports vialsof/sson POSIX and PowerShell on Windows, and aWebPortInspectorLivethat probeslocalhostto confirm whether a port serves HTTP/HTML.TerminalActivityEventin terminal.ts with a requiredrunningPortsfield; activity events are only emitted whenhasRunningSubprocessor the port list changes. Web port detection uses a TTL cache so transient probe failures don't drop previously confirmed ports.ThreadTerminalStatein terminalStateStore.ts to store arunningTerminalPortsmap per terminal, normalized and deduplicated on each update.http://localhost:<port>, and the terminal drawer tab shows a Globe icon button for the same action.runningPortsis now a required field inTerminalActivityEvent; any existing persisted or in-flight events without this field will fail schema decoding.Macroscope summarized b0bc101.