Skip to content

feat: DEV/TEST environments + OAuth scope step-up (DEVEX-719)#57

Merged
jpage-godaddy merged 16 commits into
rust-portfrom
additional-envs
Jun 11, 2026
Merged

feat: DEV/TEST environments + OAuth scope step-up (DEVEX-719)#57
jpage-godaddy merged 16 commits into
rust-portfrom
additional-envs

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Lets internal developers target DEV/TEST environments at runtime without committing anything to this OSS repo (DEVEX-719), and wires the CLI to cli-engine 0.2.1's OAuth scope step-up.

Environments (DEVEX-719)

  • New environments module is the single source of truth: resolve an env name to {api_url, client_id, auth_url, token_url} from built-ins (ote/prod) → ~/.config/gddy/environments.toml → per-env <ENV>_API_URL env vars (built-ins are overridable). This replaces the API-base-URL logic that was duplicated across env, auth, and client.
  • --env is validated against known envs (clear error listing them); env list/get/info/set route through the resolver.
  • No internal hostnames or client IDs are committed — internal endpoints live only in a developer's env vars / gitignored home-dir config.

OAuth scope step-up (consumes cli-engine 0.2.1)

  • GoDaddyAuthProvider forwards get_credential_for, so the per-env PkceAuthProvider performs scope step-up when the cached token lacks a command's required scopes.
  • api call merges the matched catalog endpoint's scopes with repeatable --scope and requests them via ctx.credential_with_scopes(...); --scope now requires a value.
  • SCOPES → DEFAULT_OAUTH_SCOPES; engine dependency bumped to 0.2.1.

Hardening (from a deep review pass)

  • api_url_for_env never returns an empty base URL (falls back to the built-in default).
  • Documented the built-in-override token-redirect trade-off in the module docs.
  • Added unit tests: resolver (derived URLs, precedence, overrides, listable/is_known), merge_required_scopes, api_url_for_env, and env list output-shape.

Verification

cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test (132 pass) against published cli-engine 0.2.1.

🤖 Generated with Claude Code

@jpage-godaddy @claude
feat: DEV/TEST environments + OAuth scope step-up (DEVEX-719)
c6317b9 
Consolidate environment->endpoint resolution into one module and let internal
developers target DEV/TEST at runtime without committing anything to this OSS
repo, and wire the CLI to cli-engine 0.2.1's OAuth scope step-up.

Environments (DEVEX-719):
- New `environments` module resolves an env to {api_url, client_id, auth_url,
  token_url} from built-ins (ote/prod) -> ~/.config/gddy/environments.toml ->
  per-env <ENV>_API_URL vars (built-ins overridable). Replaces the API-base-URL
  logic previously duplicated across env, auth, and client. No internal
  hostnames or client IDs are committed.
- `--env` validates against known envs; `env list/get/info/set` route through
  the resolver.

Scope step-up (consumes cli-engine 0.2.1):
- GoDaddyAuthProvider forwards get_credential_for so the per-env PkceAuthProvider
  performs scope step-up; SCOPES -> DEFAULT_OAUTH_SCOPES.
- `api call` merges the matched endpoint's scopes with --scope and requests them
  via credential_with_scopes; --scope now requires a value.

Hardening from deep review: api_url_for_env never returns an empty base URL;
documented the built-in-override token-redirect trade-off; added unit tests for
the resolver, scope merge, and api_url_for_env.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 19:52
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a centralized environment-resolution layer and updates auth/API call flows to support runtime DEV/TEST targeting and OAuth scope “step-up” via cli-engine 0.2.1, without committing internal endpoints into the repo.

Changes:

  • Introduces a new environments module that resolves {api_url, client_id, auth_url, token_url} from built-ins → local TOML config → per-env *_API_URL overrides.
  • Updates auth provider wiring to delegate get_credential_for to env-specific PkceAuthProvider instances, enabling scope step-up.
  • Enhances api call to merge endpoint-declared scopes with --scope flags and request credentials with those scopes; bumps cli-engine to 0.2.1.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
rust/src/main.rs Registers/validates global --env flag and wires new environment resolution.
rust/src/environments/mod.rs New environment resolver with precedence rules, derived OAuth URLs, and unit tests.
rust/src/env/mod.rs Routes env list/get/set/info through the resolver and updates output tests.
rust/src/auth.rs Reworks auth provider to build per-env PKCE providers on demand and forward scope-aware credential requests.
rust/src/application/client.rs Resolves API base URL via the new resolver with fallback behavior; adds unit tests.
rust/src/api_explorer/mod.rs Adds scope-merging helper and updates api call to request scoped credentials and improve 403 messaging.
rust/Cargo.toml Bumps cli-engine dependency to 0.2.1.
rust/Cargo.lock Updates lockfile for cli-engine 0.2.1 and transitive dependency bumps (e.g., socket2).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rust/src/main.rs
Comment thread rust/src/main.rs
Comment thread rust/src/environments/mod.rs
Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/auth.rs
@jpage-godaddy @claude
fix: address Copilot review on environments + scope step-up
c9267bd 
- main.rs: validate --env only when explicitly passed, so a malformed
  ~/.config/gddy/environments.toml no longer fails every command (the default
  env comes from .gdenv/DEFAULT_ENV and is already filtered).
- environments: normalize api_url (trim trailing slash) once during resolution
  so callers concatenating paths never produce '//'; update the test.
- auth.rs: list_environments propagates a malformed-config error instead of
  silently reporting zero environments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 20:03
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread rust/src/api_explorer/mod.rs
Comment thread rust/src/application/client.rs
Comment thread rust/src/env/mod.rs
@jpage-godaddy @claude
fix: address Copilot round 2 on PR #57
89eece4 
- api call --scope: use ArgAction::Append (one value per occurrence) instead of
  num_args(1..), which could greedily consume the required ENDPOINT positional.
- client.rs test: assert builtins resolve to a URL rather than exact hosts
  (built-ins are overridable; exact mapping covered in environments::tests).
- env list test: accept any scheme (http/https) for apiUrl shape.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 20:12
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread rust/src/api_explorer/mod.rs
Comment thread rust/src/application/client.rs
Comment thread rust/src/environments/mod.rs
@jpage-godaddy @claude
fix: address Copilot round 3 on PR #57
1834637 
- merge_required_scopes de-dupes repeated --scope flag values too (not just
  endpoint-vs-flags); added a test.
- environments::resolve falls back to built-ins + <PREFIX>_API_URL env vars when
  the optional local config is malformed/unreadable, so a broken file no longer
  bricks auth-required commands targeting prod/ote; the load error still surfaces
  for envs that actually need the file.
- client.rs test asserts URL shape ('://') rather than the https scheme.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 20:20
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/environments/mod.rs
@jpage-godaddy @claude
fix: reject empty/whitespace api_url overrides (Copilot round 4)
e9f28b5 
A blank api_url (from PROD_API_URL="" or a blank config entry) could clobber a
built-in default and yield relative request/OAuth URLs, and made is_known accept
an unusable env. Add clean_url (trim whitespace + trailing slash, None if empty);
resolve ignores empty overrides; is_known (incl. the malformed-config fallback)
requires a non-empty api_url. Added tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 20:31
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread rust/src/environments/mod.rs
Comment thread rust/src/environments/mod.rs
Comment thread rust/src/api_explorer/mod.rs
@jpage-godaddy @claude
fix: exclude empty-api_url config entries from known list + env list …
28e584d 
…(round 5)

Consistency with the round-4 empty-api_url handling: known_names no longer
advertises unusable entries, and listable_with skips them so one invalid entry
can't fail env list / credential enumeration wholesale.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 20:37
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.

Comment thread rust/src/auth.rs
Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/environments/mod.rs
Comment thread rust/src/environments/mod.rs
Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/environments/mod.rs
@jpage-godaddy @claude
fix: require http(s):// scheme for api_url overrides (Copilot round 6)
f8c599f 
clean_url is now the single validity authority: it requires an http(s):// scheme
with a non-empty host (reqwest needs absolute URLs). resolve, is_known,
known_names, and listable_with all defer to it, so a present-but-schemeless
api_url is treated as unusable consistently (rejected, not advertised, and never
fails env list / enumeration wholesale). Added a test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 21:09
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/env/mod.rs
@jpage-godaddy @claude
fix: validate auth/token overrides + show active env in list (round 7)
717e5ad 
- resolve runs auth_url/token_url config overrides through clean_url (scheme
  validation), so an invalid override falls back to the derived endpoints rather
  than failing later with an opaque reqwest error.
- env list injects the active env when it's defined only via <ENV>_API_URL (and
  thus excluded from listable), so the list always has an active:true entry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 21:18
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread rust/src/environments/mod.rs
@jpage-godaddy @claude
fix: don't hard-code config path in listable warning (Copilot)
f894447 
environments_path() resolves per-OS (XDG/%APPDATA%/macOS); the EnvError already
carries the resolved path, so the warning no longer bakes in ~/.config/...

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 22:41
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread rust/src/api_explorer/mod.rs
@jpage-godaddy @claude
fix: 403 hint repeats --scope per scope (Copilot)
35b5544 
auth login --scope is append-style (one value per flag), so the api call 403
hint now suggests `auth login --scope a --scope b` rather than space-joining.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 22:46
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread rust/src/auth.rs
@jpage-godaddy @claude
docs: fix stale list_environments comment (Copilot)
399aa06 
listable() falls back to built-ins (with a warning) on a malformed config; the
comment no longer claims the error is surfaced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 23:10
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/auth.rs Outdated
@jpage-godaddy @claude
docs: describe env config path as platform-dependent (Copilot)
be68264 
Module/schema/auth doc comments no longer hard-code ~/.config/gddy; they
reference dirs::config_dir()/environments_path() (XDG/%APPDATA%/macOS).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 23:16
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread rust/src/environments/mod.rs
@jpage-godaddy @claude
fix: case-insensitive URL scheme in clean_url (Copilot)
277349e 
URL schemes are case-insensitive (RFC 3986), but clean_url used case-sensitive
strip_prefix, so a valid override like HTTPS://host was silently dropped. Match
the scheme case-insensitively while preserving host case. Added a test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 23:44
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Comment thread rust/src/environments/mod.rs Outdated
Comment thread rust/src/env/mod.rs Outdated
@jpage-godaddy @claude
docs: correct OAuth override var names + get_env survival caveat (Cop…
2c08e03 
…ilot)

- Module doc spells the cli-engine override vars in full
  (<PREFIX>_OAUTH_AUTH_URL / _OAUTH_TOKEN_URL), not the misleading _AUTH_URL.
- get_env comment notes a config-only env is dropped if the config can't be
  read (is_known then falls back to built-ins + <ENV>_API_URL).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpage-godaddy jpage-godaddy requested a review from Copilot June 10, 2026 23:49
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

@jpage-godaddy jpage-godaddy merged commit 32cd77e into rust-port Jun 11, 2026
2 checks passed
@jpage-godaddy jpage-godaddy deleted the additional-envs branch June 11, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jgowdy-godaddy jgowdy-godaddy jgowdy-godaddy approved these changes

+1 more reviewer

@axburgess-godaddy axburgess-godaddy axburgess-godaddy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jpage-godaddy @jgowdy-godaddy @axburgess-godaddy