Skip to content

Fix/windows conpty ansi leak - #857

Open
ilyaxuwu wants to merge 2 commits into
CodebuffAI:mainfrom
ilyaxuwu:fix/windows-conpty-ansi-leak
Open

Fix/windows conpty ansi leak#857
ilyaxuwu wants to merge 2 commits into
CodebuffAI:mainfrom
ilyaxuwu:fix/windows-conpty-ansi-leak

Conversation

@ilyaxuwu

@ilyaxuwu ilyaxuwu commented Jul 9, 2026

Copy link
Copy Markdown

No description provided.

@ilyaxuwu

ilyaxuwu commented Jul 9, 2026

Copy link
Copy Markdown
Author

Hopefully this fixes Codebuff/Freebuff!

@jahooma

jahooma commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks! Coding agent thought it was a reasonable fix. Merged a version of this

@codebuff-team

Copy link
Copy Markdown
Contributor

The instinct here is reasonable — unifying process-group behavior across platforms so grandchild processes get killed consistently on timeout/abort. But the justification in the new comment is not accurate: Node's detached on Windows is implemented via libuv, which does not map to DETACHED_PROCESS. In src/win/process.c, UV_PROCESS_DETACHED sets CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB, not DETACHED_PROCESS. So the console-attachment story in the comment (detached: true + windowsHide = fully detached console) doesn't hold up against the actual implementation.

That matters here because the original code deliberately set detached: !isWindows specifically to avoid the ConPTY ANSI-leak bug this PR claims to fix. Reverting that with an incorrect rationale risks reintroducing the exact bug it's supposed to fix, or at minimum changes signal/kill semantics on Windows (new process group via CREATE_NEW_PROCESS_GROUP) without any test or repro showing the leak is actually gone.

Given the PR is only a comment + one flag change with no test coverage, no repro steps, and no evidence the ANSI-leak scenario was actually verified on Windows post-change, this needs more rigor before it can be ported: either a reproduction script/test demonstrating the leak is fixed with detached: true, or corrected reasoning that reflects libuv's actual Windows behavior (e.g., is it windowsHide/CREATE_NO_WINDOW alone doing the real work, and is detached orthogonal?).

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 20, 2026
@codebuff-team

Copy link
Copy Markdown
Contributor

Marking this stale - there has been no activity here for 44 days. It will close in 7 days unless someone comments.

This is backlog upkeep, not a verdict on the pull request. A single comment keeps it open, and anything closed this way can be reopened.

@codebuff-team codebuff-team added the stale No activity after a maintainer request; queued for closing label Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written stale No activity after a maintainer request; queued for closing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants