Skip to content

chore: remove orphaned node-pty remnants and reconcile the README (Closes #132) - #174

Merged
harsharajkumar-273 merged 1 commit into
harsharajkumar-273:mainfrom
SakethSumanBathini:chore/132-remove-orphaned-terminal-remnants
Aug 4, 2026
Merged

chore: remove orphaned node-pty remnants and reconcile the README (Closes #132)#174
harsharajkumar-273 merged 1 commit into
harsharajkumar-273:mainfrom
SakethSumanBathini:chore/132-remove-orphaned-terminal-remnants

Conversation

@SakethSumanBathini

Copy link
Copy Markdown
Contributor

Closes #132

Opening this per your go-ahead. As traced on the issue, terminalServer.ts doesn't exist on main — this removes what the deleted feature left behind, which is what caused the report.

What was left over

The terminal feature was removed from the code but not from everything describing it:

  • node-pty@^1.1.0 is in backend/package.json and imported by nothing. node-pty is a native module, so every contributor and every CI run pays to compile it for no benefit.
  • PROOFDESK_TERMINAL_MODE and PROOFDESK_TERMINAL_RUNTIME are set in docker-compose.prod.yml and read by no code.
  • README.md documents the feature as current in four places, including the architecture diagram.

That last one is why this matters beyond tidiness: #125 and #132 are both security-shaped reports filed against a file that doesn't exist, because the documentation still describes an attack surface the code doesn't have.

Change

backend/package.jsonnode-pty removed. The other 24 dependencies are untouched.

docker-compose.prod.yml — the two dead variables removed.

README.md — the terminal claims now describe the mechanism that actually ships. Build output reaches the client over Server-Sent Events: GET /build/logs/:sessionId sets Content-Type: text/event-stream (build.controller.ts:94) and BuildLogPanel.tsx:59 consumes it with EventSource. I checked that path before rewriting rather than guessing.

Four edits: the overview paragraph, the section heading and its streaming bullet, the Mermaid node, and the sandbox description. The prose around them is unchanged.

Left alone deliberately

Line 150 still reads "prune dangling Docker PTY sockets" in the roadmap. It's a future item rather than a description of current behaviour, and rewording someone's roadmap felt out of scope — say the word if you'd like it updated too.

Verification

  • backend/package.json still parses as valid JSON; 24 dependencies remain.
  • No node-pty reference survives in any of the three files.
  • No code reads either removed variable (grep across backend/src returns nothing).
  • backend/package-lock.json regenerated in the same commit and npm ci --dry-run resolves cleanly, so the lockfile and manifest stay consistent for CI.

Worth noting backend/package-lock.json needs regenerating alongside this so npm ci stays consistent — I've handled that in the same commit.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@SakethSumanBathini, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06393ae1-393a-44e9-9186-67fead115c8d

📥 Commits

Reviewing files that changed from the base of the PR and between 44cd432 and 9aa84f5.

⛔ Files ignored due to path filters (1)
  • backend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • README.md
  • backend/package.json
  • docker-compose.prod.yml

Comment @coderabbitai help to get the list of available commands.

@harsharajkumar-273 harsharajkumar-273 added ELUSOC Required Tracking ADVENTURER Intermediate (25 pts) labels Aug 4, 2026
@harsharajkumar-273
harsharajkumar-273 merged commit bfa7e0e into harsharajkumar-273:main Aug 4, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ADVENTURER Intermediate (25 pts) ELUSOC Required Tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Backend] CommonJS require() Used Directly inside ES Module (terminalServer.ts)

2 participants