Repository navigation
Fix varlock run dropping piped output on exit - #1296
Merged
Merged
Conversation
Wait for the child's piped stdout/stderr to close after it exits (bounded to 2s so a backgrounded process holding the pipe can't hang varlock), and flush our own stdout/stderr before gracefulExit, whose 1s flush cap truncated output to slow readers. Fixes #1279
Contributor
|
The changes in this PR will be included in the next version bump.
|
Contributor
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
The slow-reader fix still drops output under Bun, and the new unlimited drain can prevent signal-driven shutdown after the child exits.
Reviewed changes: Reviewed the child-pipe lifecycle, parent stdio drain, both CLI integrations, regression tests, and documentation.
- Child output lifetime: Delays
execsettlement until piped streams close, with a two-second post-exit cap for descendants. - Parent output drain: Adds a pre-exit stdout/stderr flush to
runand the shared proxy-run completion path. - Tests and documentation: Covers late descendant output and timeout behavior, and documents the bounded descendant wait.
gpt-6.1-sol ｜ 𝕏
varlock
@varlock/astro-integration
@varlock/cloudflare-integration
@varlock/nextjs-integration
@varlock/nuxt-integration
@varlock/vite-integration
@varlock/1password-plugin
@varlock/aws-secrets-plugin
@varlock/bitwarden-plugin
@varlock/keepass-plugin
@varlock/pass-plugin
@varlock/proton-pass-plugin
commit: |
Bun's stdout/stderr bypass the Writable queue, so writableLength stays 0 and an empty write's callback fires with output still queued. Track write() returning false and wait for drain instead, which works in both runtimes. After the child exits, a forwarded signal was a no-op, so a stalled reader made varlock unkillable during the unbounded flush. Exit with 128+N instead.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes: Reviewed the output-drain and signal-lifecycle fixes added since the prior review.
- Tracked backpressure: Added destination
write()/draintracking so Bun's native stdio buffer is drained before exit. - Restored termination: Made signals received after child detachment exit varlock with
128+Nacross the shared run paths. - Added regression coverage: Covered delayed readers under Node and Bun, late descendant output, and SIGTERM during a stalled drain. All four smoke tests and 12 focused unit tests passed.
gpt-6.1-sol ｜ 𝕏
… flush A signal after the child exits now waits for the caller's cleanup (proxy audit log flush, session record) instead of cutting it short. beginFinalFlush() then exits with 128+N if one arrived, and makes later signals exit immediately so a stalled reader still can't make varlock unkillable.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes: Reviewed the signal-lifecycle changes since 659348b. The build, 13 focused unit tests, and all four output-drain smoke tests passed.
- Held post-child signals until cleanup finishes, preserving the first signal's exit status.
- Added final-flush transitions to
runand both shared proxy-run paths so signals still terminate a stalled output drain. - Expanded signal tests to cover deferred cleanup termination and immediate final-flush termination.
gpt-6.1-sol ｜ 𝕏
theoephraim
added a commit
that referenced
this pull request
Nov 13, 2026
* Fix release PR smoke and framework test failures Keep the literal '_VARLOCK_ENV_KEY is not set' in the runtime missing-key error (changed in #1295), which framework tests assert on (and guard against with shouldNotContain). Skip the two run-output-drain smoke tests on Windows, where they have failed on every run since #1296. * Add empty bump file
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
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.
Fixes #1279. Supersedes #1292.
When stdout/stderr are piped (redaction on),
varlock runandvarlock proxy runcould lose output:execsettled on the child'sexitevent, which can fire before its pipes are read. Output still unread, or written by a background process the child started, was dropped. It now waits for the pipes to close, capped at 2s after exit so a backgrounded process holding the pipe (sh -c 'server &') can't hang varlock. After the cap the pipes are destroyed (previous behavior, just later).gracefulExitonly flushes stdio for 1s, so a slow reader (varlock run -- cmd | (sleep 2; wc -c)) got 64KB of a 2MB write. We now flush our own stdout/stderr first, blocking on the reader like a direct pipe would. Bun's stdio bypasses the Writable queue (writableLengthstays 0), so the flush trackswrite()returning false and waits fordrain, which works in both runtimes.beginFinalFlush()then exits with 128+N, and any signal during the flush exits immediately.No added delay in the normal case: the wait ends when the pipes close, which is at child exit unless a descendant holds them. TTY/inherited stdio has no pipes and no wait.