Skip to content

fix(cli): bound piped stdin buffering (CLI-2223) - #6290

Open
7ttp wants to merge 4 commits into
developfrom
7ttp/cli-2223-cli-stdin-buffering-can-oom-long-running-commands-fed-by-yes
Open

fix(cli): bound piped stdin buffering (CLI-2223)#6290
7ttp wants to merge 4 commits into
developfrom
7ttp/cli-2223-cli-stdin-buffering-can-oom-long-running-commands-fed-by-yes

Conversation

@7ttp

@7ttp 7ttp commented Aug 21, 2026

Copy link
Copy Markdown
Member

TL;DR

fixes the CLI being OOM killed when a long running command is fed by an unbounded pipe such as yes | supabase db push which was caused by the line reader holding stdin open but idle between prompts
and is now fixed by reading piped stdin ahead into a bounded queue instead of on demand....

what's biting the user?

Go read stdin on demand with one bufio.Scanner, which was safe because the pipe pushed back and yes simply blocked. The port kept that shape, but Bun's process.stdin cannot be throttled:
once a reader is attached Bun keeps reading the pipe, and pause, destroy, unref and detaching the listener all fail to stop it.
An idle reader therefore buffers for the whole migration, reaching 5 to 6 GB RSS in seconds. --yes escapes it only because the reader is lazy and never opens stdin....

fixed now by:

Piped stdin is drained continuously into a bounded queue of 1024 lines, which holds memory flat at around 120-140MB Overflow drops newest first, so the lines prompts read stay in pipe order.
A TTY keeps the on demand reader, since a keyboard cannot outrun the prompts and a drainer would take keystrokes a later prompt needs.....

Note

The line cap and the drop on overflow are deliberate, not an oversight
a source that honors backpressure needs no cap at all
but it replaces the platform Stdio service rather than one consumer of it, so it is better to be tracked separately.

alt approach considered: backpressured fd 0 (maybe a later followup)

Bun's process.stdin cannot be throttled, but fs.createReadStream("", { fd: 0 }) can.
Measured with the same reader logic and the same yes | producer:

source RSS under yes |, 8s idle
process.stdin, on demand (before) 5 to 6 GB, climbing
process.stdin, bounded queue (this PR) ~150 MB
fs.createReadStream("", { fd: 0 }) ~100 MB

With real backpressure nothing is read ahead, so the queue,
the cap and the drop all disappear and the TTY and pipe paths collapse back into one reader.
It is not taken here because it means providing a replacement Stdio service across its 23 provision sites,
three of which sit inline next to stdinLayer, so it touches production layer wiring rather than a single consumer....

It also does not close everything on its own.
A line with no terminator still accumulates inside splitLines and needs a separate length bound,
and a paused fd 0 stream reads far enough ahead to matter for a child later spawned with inherited stdin.
Both need settling before it can replace this...

ref:

@7ttp 7ttp self-assigned this Aug 21, 2026
@7ttp
7ttp requested a review from a team as a code owner August 21, 2026 11:24
Comment thread apps/cli/src/shared/runtime/stdin.layer.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bdc3082de

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread apps/cli/src/shared/runtime/stdin.layer.ts
Comment thread apps/cli/src/shared/runtime/stdin.layer.ts
Comment thread apps/cli/src/shared/runtime/stdin.layer.ts Outdated
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@b579f998ca30736a579477881d73f28b3193c91b

Preview package for commit b579f99.

@7ttp
7ttp marked this pull request as draft August 21, 2026 11:36
@7ttp
7ttp marked this pull request as ready for review August 21, 2026 12:13

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 474458e78b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread apps/cli/src/shared/runtime/stdin.integration.test.ts Outdated
Comment thread apps/cli/src/shared/runtime/stdin.service.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI stdin buffering can OOM long-running commands fed by yes

1 participant