The engine works out whether it is in CI, instead of asking every host - #155
Conversation
Runtime.isCI was a required field every host had to answer, and every
host answered it the same way: import ci-info, pass its isCI through.
prisma-cli's own bin did exactly that, composer's CLI had to do it
again, and a host that hand-rolled the answer instead would fork a
vendor detection table that goes stale.
The old rationale was that "the engine detects CI no more than it
detects a TTY". That comparison does not hold. TTY-ness cannot be
derived from the environment; CI-ness can, and the engine is already
handed the environment as Runtime.env.
So the engine detects it. src/ci.ts reads ci-info's vendors.json — the
upstream table itself, imported from the installed package rather than
copied into this repo, so upgrading ci-info is what teaches the engine a
new provider — and matches it against the injected env. ci-info's own
isCI export cannot be used: that module evaluates against the real
process.env the moment it is imported, which the engine may not read.
Only the ~20-line matcher is ours. It agrees with ci-info's isCI on all
53 vendors in the current table and on the CI=false, empty-CI and
partial-vendor edge cases.
Runtime.isCI becomes Runtime.isCIOverride, optional. Absence means
DETECTED, never false, so the safety property the required field existed
for still holds: a host that says nothing stays silent in CI. The
override remains for a host detection cannot serve, and for tests that
need both sides of the branch.
Interactivity now shares that detection. It used to ask whether CI was
set to anything at all, which got two things wrong: Jenkins, TeamCity
and Azure Pipelines set no CI variable and were offered prompts nobody
was there to answer, and CI=false — an explicit denial — cost a
developer their prompt. Both are fixed.
Both hosts in this repo stop answering. The v8 bin no longer imports
ci-info, and ci-info moves from @prisma/cli's dependencies to the
engine's. The test harness keeps its isCI seed and run({ isCI })
override, now wired to isCIOverride; a test that says nothing about CI
gets detection over its own env.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
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 |
commit: |
Acceptance verified against source and the merged PRs; the spec gains a Close-out section recording the two items that shipped amended (the bin's Node floor is 22.18, not 24, via composer #224; ledger Q2 was ruled dropped rather than closed by S3's mechanism). plan.md marks S3 closed and points at S8 as next. deferred.md gains the isCI item from the handover brief: composer drops its ci-info answer at its next engine-pin bump, because prisma-cli #155 made the engine detect CI itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
2 participants
A host used to have to tell the engine whether it was running in CI:
It no longer does. The engine works it out from the environment the host already hands it:
The decision
CI detection belongs to the engine.
Runtime.isCIwas required of every host, which meant every host imported the same package to compute the same boolean — and a host that wrote its own version instead forked a table of CI vendors that goes stale as new ones appear. That is what prompted this: composer, mounting its commands into this CLI, became the second host and had to answer the same question.The engine's own rationale for pushing it out was that "the engine detects CI no more than it detects a TTY." The analogy does not hold. TTY-ness is not derivable from the environment; CI-ness is, and the engine already receives
Runtime.env— the injected, process-global-free environment its own rules require it to read from.The field survives as
isCIOverride, optional. Removing it outright was tempting, but a test that needs to force not-CI would then have to doctorenv.CI, and that variable is read by other things — the interactivity decision, the telemetry opt-outs. The rename matters as much as the optionality: left asisCI?: booleanit still reads like something a host ought to answer.undefinedmeans detect, so the safety property that made it required in the first place still holds — a host that says nothing gets detection, neverfalse.How the detection avoids becoming the thing it replaces
ci-infohas noexportsmap and shipsvendors.jsonamong its files, so the upstream vendor table is importable directly. The engine reads that table and matches it against the injected env; nothing about any individual CI provider is copied into this repo. What is ours is a twenty-line matcher, and ci-info's nine conventional variable names (CI,BUILD_ID,RUN_ID, …), which live in itsindex.jsas code rather than data — cross-vendor conventions, not per-vendor facts.The matcher was checked against reality rather than reasoned about: a throwaway script compared it to real
ci-infoin child processes across all 53 vendors in the table plus seven edge cases, with no disagreements.Interactivity now shares it
The engine had a second, cruder CI test deciding whether to prompt:
runtime.env.CI === undefined. It now uses the same detection, which is better in both directions:CIvariable at all. They were being offered prompts with nobody there to answer them.CI=falseis an explicit denial, and it used to cost a developer their prompts, because the old test only asked whether the variable was set.Testing
Sixty-eight cases, built around what could go wrong rather than what the code does: a CI-looking
process.envis ignored when the injected env is clean, and the reverse; five vendors in their own env shapes, including the ones that set noCIvariable and can only be found through the table; Jenkins needing both of its variables present;CI=falseoutranking a vendor's own variables; and every vendor in the installed table detected from its own entry, so an upstream shape the matcher does not understand fails loudly rather than silently.Through whole runs: a TeamCity or Azure environment that no host declared reports no telemetry, a developer's shell does, and the override beats detection either way.
Nine existing engine tests got shorter — they deleted the
isCI: falseline they used to need, which is the change stated in one diff.🤖 Generated with Claude Code