fix(cli): migration graph --dot draws the DOT as a human block, un-redding main - #30057
Conversation
…ives one shared screen cli-engine 0.1.1 suppresses the stdout payload when stdout and stderr are one screen, treating it as a mirror of the human output. For --dot the payload was the only copy, so a terminal showed no DOT at all and main went red on the migration-graph-dot journey. The DOT is now also a human drawing block: one screen shows the human copy with the mirror suppressed, split sinks get the raw DOT on stdout too, and json mode is unchanged. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
|
|
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 (3)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe migration graph command now renders available DOT output in human mode as a drawing block. Tests verify shared-TTY stream handling and preserve DOT data in stdout presentation and JSON results. ChangesMigration graph DOT presentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change restores terminal and split-stream DOT output while preserving JSON behavior; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 📦
|
…nch window (prisma#30059) Integration Tests is no longer a required check during the launch window (until ~2026-08-21). Two changes, both marked for removal when the requirement returns: - The `test-integration` job in CI (PR) skips `merge_group` events — a queue group was spending ~30 minutes of runner time on a check that cannot block the merge. It still runs on every pull request, so the people who want the signal keep getting it. - `integration-nightly.yml` runs the same job on `main` every night at 03:00 UTC (and on `workflow_dispatch`). An unrequired check is exactly how a red e2e sat silently on main from rc.2 until prisma#30057 — the nightly run is what keeps regressions surfacing while nothing blocks on them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Integration tests now run nightly on the main branch and can also be triggered manually. - Integration tests are skipped for merge queue validation events while continuing to run for pull requests and other supported events. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…nals keep the stdout mirror (prisma#30060) ## 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. (prisma#30057 fixed the data-loss half — every payload now has a human mirror — this fixes the over-suppression half.) ## The fix `runtimeFromProcess` now compares the device and inode behind the two streams' fds (`fstatSync`) and answers the field: - same file identity → `true`: one screen, mirror suppressed, human copy shown — unchanged behavior - different identities → `false`: two sinks, both copies delivered - no fd on a stream, or fstat fails → field stays absent and the engine keeps its one-screen default, which is the safe common case This 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.ts` driving `runtimeFromProcess` with 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](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Runtime detection now reports whether standard output and error streams share the same device when this can be determined. * **Bug Fixes** * Improved handling of missing or invalid output stream descriptors by returning an indeterminate result instead of failing. * Improved configuration loading across environments with more reliable module resolution. * **Tests** * Added coverage for shared devices, separate devices, unavailable or invalid stream descriptors, and configuration loading. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
What broke
8.0.0-rc.2 shipped with
migration graph --dotprinting no DOT on a terminal, and main has been red ontest/integration/test/cli-journeys/migration-graph-dot.e2e.test.tssince the release merge (#30049) — silently, because integration tests were not a required check at the time.@prisma/cli-engine 0.1.1 treats a command's stdout payload as a mirror of the human output and suppresses it when stdout and stderr are one screen (
isTty.stdout && isTty.stderr && outputStreamsShareDevice !== false). For--dotthe payload was the only copy of the DOT text, so a terminal showed nothing, and piping switches the engine to json mode, so there was no stream shape that produced raw DOT.The fix
graphPresentationsnow also emits the DOT as a humandrawingblock. That makes the stdout payload a genuine mirror, so the engine's suppression rule is correct in every shape:dotalongside the graph documentTests
packages/1-framework/3-tooling/cli/test/orm/migration-graph.test.ts: new case pinning the one-shared-screen shape (human block present, stderr shows DOT, stdout empty, payload still declared); the split-shape case now also asserts the raw DOT actually reaches stdout.test/integration/test/cli-journeys/migration-graph-dot.e2e.test.ts: the red journey now asserts the new contract and passes.Verified locally: cli package suite (1421 tests), the DOT journey (3 tests),
pnpm typecheck,pnpm lint— all green.Follow-up not in this PR
runtimeFromProcessstill never setsRuntime.outputStreamsShareDevice, so two genuinely separate terminals are treated as one screen. Upstream prisma/prisma-cli#198 makes the field required, which will force that decision here when the next engine version is adopted.🤖 Generated with Claude Code
Summary by CodeRabbit
--dotoutput in human-readable mode.