Repository navigation
Conversation
Contributor
Package TarballHow to installgh release download pr-2750-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-1.0.1.tgz |
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
11 of 18 tasks
This branch has not been deployed
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.
Description
Read stdin through the configured process stream instead of reopening file descriptor 0.
The fd 0 path was added to work around imports draining or prematurely ending stdin under Bun. Bun fixed the paused-stdin behavior in 1.4.0, which is the version this repository now pins. Keeping the workaround causes delayed pipes to fail under Node: accessing
process.stdinmakes the pipe non-blocking, then the separate raw fd read can receiveEAGAINbefore the producer writes.This removes the obsolete workaround and adds a cross-runtime subprocess regression test. The test imports Ink, starts resolving
process.stdin, waits while the pipe is empty, and only then writes the value. It runs under both Bun and Node.This is an alternative to #2748: instead of retaining a runtime-specific branch, it verifies that the original Bun condition no longer reproduces and removes the raw fd path entirely.
Bun context:
Related Issue
Closes #2747
Documentation PR
Not applicable; command syntax and documented behavior are unchanged.
Type of Change
Testing
How have you tested the change?
bun testbun run test:e2e, or explained why they are not applicable(CI does not run them for you)
bun run typecheckbun run lint:checkbun run format:checkbun run auditbun run secrets:checkbun run buildsrc/assets/, I updated affected snapshots withbun test <test-file> --update-snapshotsand committed themby this PR
command.md(the release workflow regenerates it)Results:
bun test: 4,704 passed, 6 skipped, 0 failed across 311 files.src/io/source.test.ts: 16 passed, including delayed stdin under Bun and Node after importing Ink.EAGAIN.bun auditcould not use the configured CodeArtifact advisory endpoint (404). Against the public registry it reports one pre-existing moderateminimatchadvisory through the lint/test development toolchain; this PR changes no dependencies.The bug bash inventoried 38 handler and test files using
SourceResolver, covering 21 stdin-backed option names. Injected streams already used the configured stream directly, so the behavior change is limited to realprocess.stdin; the full suite checks the shared resolver consumers for regressions.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.