Detect interactive environment on Context#804
Draft
Amoifr wants to merge 1 commit intojolicode:mainfrom
Draft
Conversation
Add Context::isEnvInteractive() / withEnvInteractive() so tasks can know whether the surrounding environment supports interactive commands. The flag is auto-detected from the CI env var and STDIN being attached to a TTY, and can be overridden explicitly. Context::toInteractive() now throws a LogicException when the environment is not interactive, to fail fast instead of silently hanging in CI or under AI agent runners. Pass toInteractive(throwOnNonInteractiveEnv: false) to bypass the check. Refs jolicode#803
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First stab at #803: let
Contextknow whether the surrounding environment supports interactive commands, and preventtoInteractive()from silently hanging in CI / under AI agent runners.Context::isEnvInteractive()/withEnvInteractive()CIenv var + STDIN being a TTY (override withwithEnvInteractive(false))Context::toInteractive()now throws aLogicExceptionwhen the environment is not interactive, withtoInteractive(throwOnNonInteractiveEnv: false)as an escape hatchOpening as draft because two design points really benefit from your input before going further:
1. Auto-detection scope
I went conservative on purpose: only
CI+!stream_isatty(STDIN). The issue mentionedCURSOR_AGENT/AI_AGENT/ etc., but that ecosystem is fragmented and there's no real standard yet — I'd rather let users declare their own runner withwithEnvInteractive(false)than hardcode a list that goes stale fast. Happy to expand the list if you'd prefer broader detection out of the box.2. BC break on
toInteractive()As suggested by @joelwurtz in the issue,
toInteractive()now throws when the env is not interactive. This is technically a BC break for anyone callingtoInteractive()from CI today (even if such a call was almost certainly already broken in practice). The bypass param is there for the rare legitimate use case. Let me know if you'd rather have it opt-in via a separate method instead.Out of scope
The "automatically hide commands when env is not interactive" idea from the issue thread isn't in this PR — it's a bigger change touching command discovery, and felt better as a follow-up once the foundation is agreed on.
Refs #803
Test plan
vendor/bin/phpunit tests/ContextTest.php(6 new tests)tools/phpstan/vendor/bin/phpstancleantools/php-cs-fixer/vendor/bin/php-cs-fixer fixcleanRunVerboseArguments*tests + network-dependentRemoteImportRemoteTasks)Thanks a lot for maintaining Castor — happy to iterate on this based on your feedback! 🙌