chore: centralize interactive terminal detection#1000
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
d7411ee to
bb8aeb5
Compare
|
CI is blocked on Unit Tests and Coverage. The failed assertions look tied to the centralized CI-environment detection change: src/tests/update-check.test.ts no longer sees the cached upgrade notice or stale-cache background spawn, src/utils/tests/daemon-client.test.ts no longer sees the Installing... stderr notice, and src/replay/test/tests/reporters-default.test.ts still gets no cursor-hide write for the tty progress case. Please update the detection/test setup so the intended non-CI vs CI behavior is explicit, then rerun the failing checks. |
1896c21 to
ac835e8
Compare
ac835e8 to
30b3a96
Compare
30b3a96 to
9e71fff
Compare
| function isCI(env: EnvMap = process.env): boolean { | ||
| if (env.CI === 'false' || env.CI === '0') return false; | ||
| return ( | ||
| CI_ENV_MARKERS.some((name) => Boolean(env[name])) || | ||
| CI_TRUE_MARKERS.some((name) => env[name] === 'true') | ||
| ); |
There was a problem hiding this comment.
🚩 CI='false' now overrides all other CI markers, changing auth behavior on GitHub Actions
The new isCI function at src/utils/env-map.ts:20 returns false immediately when CI='false' or CI='0', before checking any other markers like GITHUB_ACTIONS or BUILDKITE. This means { CI: 'false', GITHUB_ACTIONS: 'true' } is now treated as non-CI (tests at src/utils/__tests__/env-map.test.ts:13-14 confirm this). The old isCi in src/cli/auth-session.ts (removed at old lines 485-487) checked env.GITHUB_ACTIONS === 'true' independently, so it would have returned true (CI) in that scenario. In practice this means if a GitHub Actions workflow explicitly sets CI=false (some do for build tools), detectAuthMode would now return 'local-browser' or 'device-code' instead of 'non-interactive', potentially attempting to open a browser on a CI runner. The tests assert this is intentional, but it's a meaningful behavioral change worth confirming with the team.
Was this helpful? React with 👍 or 👎 to provide feedback.
9e71fff to
55d06be
Compare
Summary
Adds a shared isInteractive(stream, env) helper for repeated CI + TTY guards and reuses it across auth, install/upload progress, update notifier, and replay reporter interactivity checks.
CI detection is kept internal to the helper and follows ci-info-style markers with explicit CI=false and CI=0 opt-outs. CI-sensitive tests now opt out explicitly with CI=0 so GitHub Actions vendor markers do not suppress expected interactive behavior. The branch stays scoped to environment/interactivity guard cleanup.
Validation
Verified with focused unit coverage for env-map, auth, replay reporter, daemon client, and update-check call sites; pnpm check:quick; pnpm format; and a final pnpm check:unit pass including smoke tests before the test-only CI setup adjustment.