Skip to content

fix(codeflow): [CI/rebase] Terminal Sessions Memory Leak #43

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.

Sign up for GitHub

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

Open
nehraa wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(codeflow): [CI/rebase] Terminal Sessions Memory Leak #43

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/codeflow-store/src/shared/terminal-sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const IDLE_TIMEOUT_MS = 30 * 60 * 1000; // 30 min — sessions idle this long ar
const EXPIRED_OUTPUT_GRACE_MS = 60 * 1000; // 60 s — after exit, retain snapshot for this long before deletion
const PURGE_INTERVAL_MS = 30 * 1000; // 30 s — background sweep cadence
const SIGKILL_TIMEOUT_MS = 5 * 1000; // 5 s — SIGKILL escalation if SIGTERM is ignored
const MAX_SESSIONS = 50; // hard cap on concurrent sessions to bound resource use

@qodo-code-review qodo-code-review Bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Session limits never reach users 🐞 Bug ≡ Correctness

terminal-sessions.ts is absent from the package exports and has no production import; only its
colocated test imports it directly. The user-facing terminal panel updates local display state
instead, so the new guard is never executed through shipped terminal behavior.
Agent Prompt 
## Issue description
The new terminal-session limit is implemented in a module that is neither exported nor invoked by production code, so it cannot remediate shipped terminal behavior.

## Fix Focus Areas
- packages/codeflow-store/src/shared/terminal-sessions.ts[40-40]
- packages/codeflow-store/package.json[8-52]
- packages/Codeflow_master/src/components/panels/TerminalPanel.tsx[79-92]

## Recommended Fix
Apply the lifecycle and limit enforcement in the terminal implementation actually invoked by users, then expose and call that implementation through a supported package export and production route rather than importing test-only source directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


const sessions = new Map<string, InternalTerminalSession>();
let sessionCounter = 0;
Expand Down Expand Up @@ -245,6 +246,14 @@ export const createTerminalSession = async (options?: {
cwd?: string;
title?: string;
}): Promise<TerminalSessionSnapshot> => {
// Reap idle/expired sessions first so the cap reflects live sessions only.
purgeIdleSessions();
if (sessions.size >= MAX_SESSIONS) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude exited sessions from the concurrent-session cap

After 50 shells terminate naturally, their entries remain in sessions for the 60-second output grace period because purgeIdleSessions retains exited and error sessions until then. This sessions.size check still counts those non-running entries, so every new session creation fails during that period even when no terminal is concurrent. Count only running sessions for this limit or release capacity when a child exits.

Useful? React with 👍 / 👎.

Comment on lines +250 to +251

@qodo-code-review qodo-code-review Bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Concurrent requests bypass the cap 🐞 Bug ☼ Reliability

createTerminalSession checks sessions.size without reserving a slot, then yields while awaiting
resolveInitialCwd before registering the new session. Concurrent calls can therefore all pass
below 50 and subsequently spawn and retain more than the intended maximum number of child processes.
Agent Prompt 
## Issue description
The session limit check is separated from registration by asynchronous work, allowing concurrent creations to exceed the hard resource cap.

## Fix Focus Areas
- packages/codeflow-store/src/shared/terminal-sessions.ts[249-257]
- packages/codeflow-store/src/shared/terminal-sessions.ts[259-304]

## Recommended Fix
Atomically reserve a session slot before the first await, include reservations in the cap calculation, and release the reservation on every validation or spawn failure. Convert the reservation into the registered session only after successful initialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +249 to +251

@qodo-code-review qodo-code-review Bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Idle sessions block new terminals 🐞 Bug ≡ Correctness

createTerminalSession invokes purgeIdleSessions but immediately counts every remaining map
entry, while that purge only signals idle running sessions and deliberately leaves them mapped. At
50 idle sessions, creation still throws until child closure and the subsequent output-grace expiry,
even though all sessions have already been selected for reaping.
Agent Prompt 
## Issue description
Sessions already selected for idle termination remain counted by the newly added capacity check and temporarily prevent replacement sessions from opening.

## Fix Focus Areas
- packages/codeflow-store/src/shared/terminal-sessions.ts[188-221]
- packages/codeflow-store/src/shared/terminal-sessions.ts[249-255]

## Recommended Fix
Base process capacity on running sessions that have not been selected for termination, or remove reaped entries from the capacity accounting immediately while retaining any required output snapshots separately. Keep pending creation reservations in the same capacity calculation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

throw new Error(
`Terminal session limit reached (${MAX_SESSIONS}). Close an existing session before opening a new one.`
Comment on lines +249 to +253

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reserve a slot before awaiting the working directory

When many requests create sessions simultaneously while the map has room, every call performs this size check and then suspends at await resolveInitialCwd before any session is inserted. Once those awaits complete, every call spawns and inserts a child, so a burst can exceed 50 by an arbitrary amount and bypass the resource cap. Reserve capacity before the await, or perform the check after it.

Useful? React with 👍 / 👎.

);
}

const cwd = await resolveInitialCwd(options?.cwd);
const shell = getShellPath();
const child = spawn(shell, [], {
Expand Down