fix(cli): answer outputStreamsShareDevice from the fds so split terminals keep the stdout mirror - #30060
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe CLI detects whether stdout and stderr share a device and inode. The config loader resolves ChangesOutput stream device detection
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change corrects stdout mirroring when output streams target different terminals, while preserving the existing single-screen behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma/orm-extension-arktype-json
@prisma/orm-extension-middleware-cache
@prisma/orm-extension-paradedb
@prisma/orm-extension-pgvector
@prisma/orm-extension-postgis
@prisma/orm-extension-supabase
@prisma/orm-family-mongo
@prisma/orm-family-sql
@prisma/orm-framework
@prisma/orm-mongo
@prisma/orm-postgres
@prisma/orm-sqlite
@prisma/orm-target-mongo
@prisma/orm-target-postgres
@prisma/orm-target-sqlite
@prisma/orm-toolchain
commit: |
size-limit report 📦
|
…nals keep the stdout mirror The engine suppresses the stdout payload when both output streams are TTYs unless the host says they are separate devices. The bin never answered, so stdout and stderr routed to two different terminals were still treated as one screen and the mirror dropped. runtimeFromProcess now compares the device and inode behind fds 1 and 2 and sets the field; where a stream exposes no fd or cannot be stat-ed the answer stays absent and the engine keeps its one-screen default. Upstream prisma/prisma-cli#198 makes the field required, so this also pre-empts the next engine adoption. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
8f059fb to
f2278b8
Compare
… loading survives symlink-preserving resolution Under pnpm, resolving the bare c12 specifier can pin c12 at its symlinked node_modules path: Node's synchronous ESM linker (and the resolver state the CLI process ends up in) skips the realpath step. From the symlink path c12's own dependencies (dotenv) do not resolve, and every config load dies with CONFIG.EVALUATION_FAILED before evaluating anything. Which linker serves the import depends on the bundler's chunk graph, so any unrelated source change in the cli package could flip it — the previous commit did, turning five CI checks red while main stayed green by luck. Resolving the entry with the CommonJS resolver, realpathing it, and importing that file URL anchors c12 and all its transitive dependencies at their real on-disk locations in every resolver state. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
The Integration/E2E/Fixtures/Test failures were real but not what they looked like. The first commit's one-line source change flipped tsdown's chunk graph in the cli package, which changed which of Node's module linkers serves config-loader's The second commit fixes the class: config-loader now resolves c12's entry with the CommonJS resolver, realpaths it, and imports that file URL, anchoring c12 and its transitive deps at their real locations in every resolver state. Verified locally: repeated |
What this closes
The last piece of the cli-engine 0.1.1 stream-suppression adoption. The engine drops a command's stdout payload when both output streams are TTYs, unless the host says they are separate devices via
Runtime.outputStreamsShareDevice. Our bin adapter never set the field, so stdout and stderr routed to two different terminals were still treated as one screen and the machine-readable stdout copy was dropped. (#30057 fixed the data-loss half — every payload now has a human mirror — this fixes the over-suppression half.)The fix
runtimeFromProcessnow compares the device and inode behind the two streams' fds (fstatSync) and answers the field:true: one screen, mirror suppressed, human copy shown — unchanged behaviorfalse: two sinks, both copies deliveredThis also pre-empts prisma/prisma-cli#198, which makes the field required — when the next engine version lands, this adapter already answers.
Tests
Four new cases in
packages/1-framework/3-tooling/cli/test/orm/cli.test.tsdrivingruntimeFromProcesswith real fds: same file twice →true, two files →false, fd-less streams → absent, unstat-able fd → absent.Verified locally: cli package suite (1425 tests),
pnpm typecheck,pnpm lint— all green.🤖 Generated with Claude Code
Summary by CodeRabbit