Repository navigation
fix(flutter_tools): check runtime env vars before cached bot state - #190978
Conversation
github-actions
Bot
added
the
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies BotDetector to check environment variables before returning a cached isRunningOnBot value, allowing runtime environment variables to override a cached false state, and adds corresponding tests. Feedback points out that caching isRunningOnBot as false will bypass the Azure detector (which relies on an HTTP request rather than environment variables) on subsequent runs, preventing it from detecting an Azure environment.
zanderso
left a comment
zanderso
left a comment
There was a problem hiding this comment.
A bunch of existing test expectations are now checking that expect(persistentToolState.isRunningOnBot, isNull). The new test expectations for persistentToolState.isRunningOnBot all check for isFalse. Are there any remaining tests that still check that persistentToolState.isRunningOnBot is correctly toggled to true when it should be?
Yes! Lines 113–140 in The env var tests now check for |
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/190978, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
github-actions
Bot
added
platform-ios
|
It looks like you merged some unrelated changes into this PR. |
When container images (e.g. docker build) or CI machine templates run
flutter doctor during image construction, BotDetector evaluates
isRunningOnBot = false (as no CI environment variables are present)
and writes {"is-bot": false} to tool_state.
When that container subsequently runs inside a CI pipeline (e.g.
GitHub Actions with GITHUB_ACTIONS=true or CI=true), the cached
false was returned immediately, completely bypassing runtime
environment variable checks.
* Reorder BotDetector.isRunningOnBot to evaluate in-memory environment
variables before reading cached persistent state.
* Add unit tests verifying environment variable overrides take precedence
over cached persistent state.
…tent tool state
When container images (e.g. docker build) or CI machine templates run
flutter doctor during image construction, BotDetector evaluated
isRunningOnBot = false (as no CI environment variables were present)
and wrote {"is-bot": false} to tool_state.
When that container subsequently executed inside a CI pipeline (e.g.
GitHub Actions with GITHUB_ACTIONS=true or CI=true), the cached
false was returned immediately from disk, completely bypassing
runtime environment variable checks.
Furthermore, in-memory environment variables (CI, GITHUB_ACTIONS,
TRAVIS, BORG_ALLOC_DIR, etc.) are fast lookups that reflect transient
process/subshell context and must not be persisted to disk, as writing
is-bot: true on encountering CI=true would permanently poison a
developer's workstation cache for subsequent interactive commands.
* Reorder BotDetector.isRunningOnBot to evaluate in-memory environment
variables (BOT=false, CI, GITHUB_ACTIONS, TRAVIS, CIRRUS_CI,
etc.) before reading cached persistent state.
* Remove persistent state mutations from in-memory environment checks,
restricting PersistentToolState.isRunningOnBot caching strictly to
AzureDetector (to avoid repeated network timeout loops on non-Azure
machines).
* Add unit tests verifying environment variable overrides take precedence
at runtime without mutating or poisoning persistent disk state.
… DevOps * Add TF_BUILD check to BotDetector.isRunningOnBot to explicitly detect Azure Pipelines / DevOps CI runners in memory without probing IMDS. * Add unit test in bot_detector_test.dart.
30bf7c0 to
73a9f4d
Compare
|
@zanderso sorry. fixed |
github-actions
Bot
removed
platform-ios
zanderso
left a comment
zanderso
left a comment
There was a problem hiding this comment.
Please monitor telemetry from master channel for anomalies after landing.
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/190978, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
3 participants
Description
BotDetectorcurrently latches detection results to persistent disk state (tool_state), causing two issues:flutter doctorduring build,BotDetectorwrites{"is-bot": false}to disk. When executed in CI (GITHUB_ACTIONS=true/CI=true), the cachedfalseshort-circuits evaluation, bypassing runtime CI environment variables.tool_statefor future interactive runs.Changes
BotDetector.isRunningOnBotto evaluate runtime env vars (BOT=false,CI,GITHUB_ACTIONS,TRAVIS,BORG_ALLOC_DIR, etc.) before reading persistent cache._persistentToolState.setIsRunningOnBotfrom in-memory checks._persistentToolState.isRunningOnBotnow only caches_azureDetector.isRunningOnAzureto avoid 250ms–1s timeout stalls on non-Azure machines.bot_detector_test.dartverifying runtime env overrides and cache isolation across runs.