Skip to content

feat(providers): expose actionable OAuth refresh failures - #2887

Merged
johntmyers merged 5 commits into
NVIDIA:mainfrom
mrunalp:2886-oauth-refresh-actionable-errors/mrunalp
Aug 25, 2026
Merged

feat(providers): expose actionable OAuth refresh failures#2887
johntmyers merged 5 commits into
NVIDIA:mainfrom
mrunalp:2886-oauth-refresh-actionable-errors/mrunalp

Conversation

@mrunalp

Copy link
Copy Markdown
Collaborator

Summary

Expose structured, provider-neutral recovery guidance for OAuth refresh failures so consumers can distinguish transient retries, operator configuration problems, and user reauthorization without parsing provider error text. Preserve safe diagnostics, stop rapid retries for terminal grants, and keep the existing credential-driver storage boundary unchanged.

Related Issue

Closes#2886

Changes

  • Parse bounded OAuth error responses and persist stable recovery actions, failure codes, safe provider subtypes, and failure timestamps.
  • Park refresh grants that require user reauthorization, retry configuration failures hourly, and retain short retries for transient failures and rotated-token persistence recovery.
  • Surface actionable refresh status through the CLI and curated Go SDK, including correct handling of the parked-refresh next-time sentinel.
  • Add unit coverage for classification, persistence, cleared success state, scheduling, response bounds, provider gates, and post-mint failures.
  • Add a rootless Podman + Keycloak E2E lane that uses the default database credential store and verifies refresh, revocation, and reauthorization status.
  • Document the recovery contract in provider docs, gateway architecture, protobuf comments, and the OpenShell CLI skill.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)
  • cargo test -p openshell-server --lib (1,430 passed, 8 ignored)
  • mise run go:ci
  • mise run e2e:provider-refresh-keycloak

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@johntmyersjohntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

This PR is project-valid through accepted issue #2886. The initial review found one blocking OAuth error-classification defect; the provider docs and CLI skill updates otherwise cover the direct UX contract.

Action required: @mrunalp, preserve recognized top-level OAuth classifications when an optional subtype has an unexpected JSON type, and add the malformed-subtype regression case.

Blocking findings:

  • GATOR-88e83828-01: a non-string optional subtype discards a recognized invalid_grant classification

Carried findings:

  • None

Non-blocking suggestions:

  • None
Gator metadata
  • Validation: Implements accepted issue #2886 across the existing provider-refresh boundary
  • Docs: Fern provider docs, gateway architecture, protobuf comments, and CLI skill updated
  • Checks: DCO and vouch are green; required branch and E2E workflows are not dispatched for this head yet
  • E2E: test:e2e required for provider credential flow; dispatch deferred until blocking review feedback is resolved
  • Head SHA: 88e8382897a857893f1a9dbba824e3a486819960
  • Base SHA: 2f7fb65591ee5746217ace154afc0aa6bf1afc6d
  • Merge base SHA: 2f7fb65591ee5746217ace154afc0aa6bf1afc6d
  • Patch ID: 9f514aa29504caf111a75d371d6c06023ccbea9d
  • Gator payload: 6
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

Comment threadcrates/openshell-server/src/provider_refresh.rs
@johntmyersjohntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Aug 23, 2026
@mrunalp
mrunalpforce-pushed the 2886-oauth-refresh-actionable-errors/mrunalp branch from 1b3ab0e to ad9591eCompareAugust 24, 2026 16:43
@mrunalpmrunalp added the test:e2e Requires end-to-end coverage label Aug 24, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/2887 does not exist yet. A maintainer needs to comment /ok to test ad9591ef2f78c121db2e950dd1f6fbc65077568e to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

@mrunalp

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test ad9591e

@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test ad9591e

@johntmyersjohntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Thanks @mrunalp. I checked your fix for the malformed OAuth subtype case: the top-level invalid_grant classification now survives an object-valued optional subtype, and the exact regression case verifies reauthorize with a parked retry schedule. The prior finding is resolved, and the bounded follow-up review found no new blocking issues.

Blocking findings:

  • No blocking findings remain

Carried findings:

  • None; GATOR-88e83828-01 is resolved
Gator metadata
  • Validation: Implements accepted issue #2886 across the existing provider-refresh boundary
  • Docs: Fern provider docs, gateway architecture, protobuf comments, and CLI skill are updated
  • Checks: Current-head Branch Checks and Helm Lint are running
  • E2E: test:e2e is applied; /ok to test ad9591ef2f78c121db2e950dd1f6fbc65077568e created the current mirror and Branch E2E Checks run 32764400518 is running
  • Head SHA: ad9591ef2f78c121db2e950dd1f6fbc65077568e
  • Base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
  • Merge base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
  • Patch ID: df8405233cd23b57f75f0729f664d0b43b0dda81
  • Gator payload: 7
  • Review mode: follow_up
  • Previous reviewed SHA: 88e8382897a857893f1a9dbba824e3a486819960
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:watch-pipeline

@johntmyersjohntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for ad9591e. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
mrunalpforce-pushed the 2886-oauth-refresh-actionable-errors/mrunalp branch from ad9591e to 7839728CompareAugust 25, 2026 15:58
@mrunalp

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test 7839728

@johntmyersjohntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:blocked Gator is blocked by process or repository gates labels Aug 25, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Maintainer Approval Needed

Gator validation and PR monitoring are complete. The current head is rebase-equivalent to the reviewed patch, the prior OAuth subtype finding remains resolved, and no blocking review feedback remains.

Review: The accepted issue-backed provider refresh change completed two review rounds; the current patch ID matches the last reviewed patch.

Human maintainer approval is now required.

Gator metadata
  • Validation: Implements accepted issue feat(providers): expose actionable OAuth refresh failures #2886 across the existing provider-refresh boundary
  • Docs: Provider docs, gateway architecture, protobuf comments, and the CLI skill are updated
  • Checks: OpenShell / Branch Checks and OpenShell / Helm Lint are green
  • E2E: test:e2e is applied and OpenShell / E2E is green, including the provider-refresh Keycloak lane
  • Head SHA: 7839728318528f76fef465b494a8b7cd6ac97e18
  • Patch ID: df8405233cd23b57f75f0729f664d0b43b0dda81
  • Gator payload: 7
  • Review mode: already_reviewed (rebase-equivalent)
  • Next state: gator:approval-needed

@johntmyersjohntmyers added gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Aug 25, 2026
@johntmyersjohntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Aug 25, 2026
@johntmyers
johntmyers added this pull request to the merge queueAug 25, 2026
Merged via the queue into NVIDIA:main with commit 18ce13bAug 25, 2026
72 of 75 checks passed
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: PR #2887 reached gator:merge-ready with maintainer approval before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Gator metadata
  • Head SHA: 7839728318528f76fef465b494a8b7cd6ac97e18
  • Gator payload: 7
  • Final state: merged

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2eRequires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(providers): expose actionable OAuth refresh failures

2 participants

@mrunalp@johntmyers