fix: honour MOBILERUN_OAUTH_MANUAL across OAuth providers (part of #396) - #436
Open
8crsk wants to merge 1 commit into
Open
fix: honour MOBILERUN_OAUTH_MANUAL across OAuth providers (part of #396)#4368crsk wants to merge 1 commit into
8crsk wants to merge 1 commit into
Conversation
…om telemetry docs Part of droidrun#396 (droidrun -> mobilerun rename). The rename gave every user-facing env var a MOBILERUN_* spelling with the legacy DROIDRUN_* name kept as a fallback -- MOBILERUN_CONFIG, MOBILERUN_TELEMETRY_ENABLED and MOBILERUN_STREAM_SCREENSHOTS all do this. OAUTH_MANUAL was missed, so it remained readable only as DROIDRUN_OAUTH_MANUAL across all three OAuth providers. A user following the current docs had no working way to force the manual/headless login flow. - Read MOBILERUN_OAUTH_MANUAL first, falling back to DROIDRUN_OAUTH_MANUAL, in the Anthropic, Gemini and OpenAI login paths. - Document the variable in docs/sdk/configuration.mdx. - Fix telemetry docstrings and the user-facing "telemetry disabled" message, which still told users to set DROIDRUN_TELEMETRY_ENABLED even though the code prefers MOBILERUN_TELEMETRY_ENABLED and the docs already use it. - Add tests covering both spellings, precedence, and falsy/unset values. Deliberately left alone, since renaming them would break existing installs rather than finish the rename: - compat/droidrun/, the intentional backwards-compat shim package - ~/.droidrun/user_id and the "droidrun" platformdirs app name (on-disk state; renaming orphans existing users' config and telemetry ids) - droidrun.* OpenTelemetry span and attribute names (wire format consumed by existing dashboards) - github.com/droidrun/mobilerun URLs, which are the live repo paths
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
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.
Part of #396.
While working out what's left of the
droidruntomobilerunrename I found oneuser-facing environment variable that got missed. It's a functional gap rather
than a cosmetic one.
The gap
The rename gave every user-facing env var a
MOBILERUN_*spelling, keeping thelegacy
DROIDRUN_*name as a fallback:MOBILERUN_*supported?MOBILERUN_CONFIGDeprecationWarningfor the old nameMOBILERUN_TELEMETRY_ENABLEDMOBILERUN_STREAM_SCREENSHOTSOAUTH_MANUALDROIDRUN_OAUTH_MANUALonlyOAUTH_MANUALis read in all three OAuth login paths and each one only checkedDROIDRUN_OAUTH_MANUAL. Anyone settingMOBILERUN_OAUTH_MANUAL, which is thespelling consistent with every other documented variable, silently got the
interactive browser flow instead. On a headless box that
_is_headless_environment()doesn't catch, there was no working way to force thecopy/paste login.
What this changes
MOBILERUN_OAUTH_MANUALfirst and falls back toDROIDRUN_OAUTH_MANUAL,matching the pattern in
ConfigLoader.loadandis_telemetry_enabled.docs/sdk/configuration.mdx. It wasn't documentedunder either name.
which still told people to set
DROIDRUN_TELEMETRY_ENABLEDeven though the codeprefers
MOBILERUN_TELEMETRY_ENABLEDand the docs already use the new name.What I deliberately didn't rename
#396 reads like it could be a repo-wide find and replace, but a mechanical rename
would break existing installs. These
droidrunreferences look load-bearing tome, so I left them alone:
compat/droidrun/, the intentional backwards-compat shim package.~/.droidrun/user_idand the"droidrun"platformdirs app name. That's on-diskstate, so renaming it orphans every existing user's config file and resets their
anonymous telemetry id. Feels like it wants a migration step rather than a
rename.
droidrun.*OpenTelemetry span and attribute names(
droidrun.screenshot,droidrun.vision.enabled,droidrun_version), sincethat's wire format already being consumed by existing dashboards.
github.com/droidrun/mobilerunURLs, because the org is stilldroidrun.Happy to do the on-disk config and telemetry migration as a separate PR if you
want it. It needs a real migration path so it seemed worth keeping out of this
one.
Tests
tests/test_oauth_manual_env_var.py, 45 cases across all three providers: bothspellings, precedence when both are set, and falsy or unset values. I checked they
fail (18 failures) with the source change reverted.
Full suite: 643 passed, with the same 4 pre-existing Windows encoding failures
that occur on a clean
main.ruff,blackandbanditare clean.