Skip to content

Convert 401 responses to PluginAuthenticationFailed#8

Merged
gmr merged 2 commits intomainfrom
feature/oidc-refresh-resilience
May 9, 2026
Merged

Convert 401 responses to PluginAuthenticationFailed#8
gmr merged 2 commits intomainfrom
feature/oidc-refresh-resilience

Conversation

@gmr
Copy link
Copy Markdown
Contributor

@gmr gmr commented May 9, 2026

Summary

Installs an httpx response hook on the deployment client so any HTTP 401 surfaces as the new PluginAuthenticationFailed instead of propagating as a generic httpx.HTTPStatusError. The host's retry layer catches this, refreshes the actor's identity connection, and retries the call once before failing the user-visible request.

Other status codes still flow through raise_for_status (or per-call error swallowing) unchanged.

Why

When a user's GitHub OAuth access token expires between the sweeper's last refresh and a request — or when the GitHub OAuth App was reconfigured to expire tokens after the user connected — calls like trigger_deployment come back as 401 and currently fail the request. With the new error type, the host can refresh the connection's refresh token and retry once, recovering transparently.

Dependencies

  • Depends on Add PluginAuthenticationFailed error imbi-common#88 for PluginAuthenticationFailed
  • pyproject.toml is temporarily pinned to feature/oidc-refresh-resilience of imbi-common; reset to remove the [tool.uv.sources] block before merging (matches the pattern used in imbi-api)

Test plan

  • New AuthenticationFailureTestCase covers 401 on repo GET and 401 on workflow dispatch
  • Full test suite passes (40 tests)

Installs an httpx response hook on the deployment client so any
HTTP 401 surfaces as the new PluginAuthenticationFailed instead of
propagating as a generic httpx.HTTPStatusError. The host's retry
layer catches this, refreshes the actor's identity connection, and
retries the call once before failing the user-visible request.

Other status codes still flow through raise_for_status (or per-call
error swallowing) unchanged.

Temporarily pins imbi-common to feature/oidc-refresh-resilience so
PluginAuthenticationFailed resolves before the imbi-common change
merges; reset to main before merging this PR.

Depends on AWeber-Imbi/imbi-common#88.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8a54d36a-e025-4cbc-8247-2fc3102be93c

📥 Commits

Reviewing files that changed from the base of the PR and between 3791e44 and 0059d53.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of authentication failures for GitHub API interactions so authentication errors trigger re-auth flows instead of generic failures.
  • Tests

    • Added tests verifying authentication failures are detected and surfaced during repository and workflow operations.
  • Chores

    • Updated project configuration to change where a shared dependency is sourced from.

Walkthrough

Adds an httpx response hook that consumes HTTP 401 responses and raises PluginAuthenticationFailed, registers the hook on the GitHub AsyncClient, updates pyproject.toml to source imbi-common from its Git repo at rev="main", and adds tests asserting 401s produce PluginAuthenticationFailed.

Changes

HTTP 401 Authentication Failure Handling

Layer / File(s) Summary
Dependency Source Configuration
pyproject.toml
[tool.uv.sources] entry added to install imbi-common from https://github.com/AWeber-Imbi/imbi-common.git at rev = "main".
Authentication Hook Implementation
src/imbi_plugin_github/deployment.py
Import PluginAuthenticationFailed; add async response hook _raise_on_401 that reads 401 response body and raises PluginAuthenticationFailed; register hook on AsyncClient via event_hooks={'response': [_raise_on_401]}.
Authentication Failure Tests
tests/test_deployment.py
Import PluginAuthenticationFailed; add AuthenticationFailureTestCase with tests that mock 401 responses for list_refs and trigger_deployment and assert PluginAuthenticationFailed is raised.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A little hook sat by the gate,
When 401s arrive, it won't wait,
It reads the reply, raises a bell,
So auth can wake and mend what's fell,
Tests nod and cheer — all done just great.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: converting HTTP 401 responses to PluginAuthenticationFailed exceptions.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation, implementation approach, and dependencies for handling 401 authentication failures.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/oidc-refresh-resilience

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyproject.toml`:
- Around line 126-127: Remove the temporary git source override block
([tool.uv.sources] imbi-common) from pyproject.toml and update the imbi-common
dependency (currently imbi-common>=0.8.2) to the first released package version
that contains PluginAuthenticationFailed (the symbol referenced by deployment.py
at the import/usage sites around lines 43 and 74); determine which released tag
or PyPI version corresponds to the feature/oidc-refresh-resilience commit and
set that as the new minimum version so non-uv package managers will pull the
release that includes PluginAuthenticationFailed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 665a3c39-44d7-4e0c-980b-111d0c6d0362

📥 Commits

Reviewing files that changed from the base of the PR and between fd0df8f and 3791e44.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • pyproject.toml
  • src/imbi_plugin_github/deployment.py
  • tests/test_deployment.py

Comment thread pyproject.toml Outdated
Comment on lines +126 to +127
[tool.uv.sources]
imbi-common = { git = "https://github.com/AWeber-Imbi/imbi-common.git", rev = "feature/oidc-refresh-resilience" }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
import json, urllib.request
url = "https://pypi.org/pypi/imbi-common/json"
data = json.load(urllib.request.urlopen(url))
print("Latest imbi-common on PyPI:", data["info"]["version"])
print("Verify first release containing:")
print("  imbi_common.plugins.errors.PluginAuthenticationFailed")
print("Then set that as the minimum in pyproject.toml and remove [tool.uv.sources].")
PY

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 2656


🏁 Script executed:

# Verify the pyproject.toml file to confirm the git source override and minimum version
cat -n pyproject.toml | grep -A 10 -B 5 "uv.sources\|imbi-common"

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 1493


🏁 Script executed:

# Check the deployment.py file for the actual import and usage of PluginAuthenticationFailed
find . -name "deployment.py" -type f -exec cat -n {} \; | head -100

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 3698


🏁 Script executed:

# Search for all references to PluginAuthenticationFailed in the codebase
rg "PluginAuthenticationFailed" -n -A 2 -B 2

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 2658


🏁 Script executed:

# Look for lock files that might show resolved versions
find . -name "*.lock" -o -name "requirements*.txt" -o -name "uv.lock" -type f

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 84


🏁 Script executed:

# Check git history for recent changes to pyproject.toml or dependency information
git log --oneline -20 -- pyproject.toml 2>/dev/null || echo "No git history available"

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 134


🏁 Script executed:

# Search for any documentation or comments about the feature branch or PluginAuthenticationFailed
rg "oidc-refresh-resilience|PluginAuthenticationFailed" -i --type md -A 2 -B 2

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 56


🏁 Script executed:

# Check if there's a CHANGELOG or version history that documents PluginAuthenticationFailed
find . -iname "CHANGELOG*" -o -iname "HISTORY*" -o -iname "CHANGES*" | head -5

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 56


🏁 Script executed:

# Check the uv.lock file for imbi-common information
rg "imbi-common|name = \"imbi-common\"" -A 10 -B 2 uv.lock | head -50

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 1540


🏁 Script executed:

# Also check if there's any version info in the git branch override
git branch -a 2>/dev/null | grep "oidc-refresh-resilience" || echo "Branch info not available"

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 100


🏁 Script executed:

# Search for any version bumps or release notes related to imbi-common
rg "0\.8\.[0-9]|0\.9\.|1\.0\." pyproject.toml

Repository: AWeber-Imbi/imbi-plugin-github

Length of output: 98


Remove the temporary git source override before merge and align the minimum imbi-common version.

Line 126–127 adds a git source override pinned to the feature/oidc-refresh-resilience branch. With dependencies still declaring imbi-common>=0.8.2 from PyPI, consumers using package managers other than uv can resolve a release that may not include PluginAuthenticationFailed (imported and used in deployment.py, lines 43 and 74), causing runtime import failures outside this override context.

Remove [tool.uv.sources] before merge and bump imbi-common to the first released version containing PluginAuthenticationFailed. The feature branch currently resolves to 0.8.2 from a specific commit; determine the corresponding released version and update pyproject.toml accordingly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyproject.toml` around lines 126 - 127, Remove the temporary git source
override block ([tool.uv.sources] imbi-common) from pyproject.toml and update
the imbi-common dependency (currently imbi-common>=0.8.2) to the first released
package version that contains PluginAuthenticationFailed (the symbol referenced
by deployment.py at the import/usage sites around lines 43 and 74); determine
which released tag or PyPI version corresponds to the
feature/oidc-refresh-resilience commit and set that as the new minimum version
so non-uv package managers will pull the release that includes
PluginAuthenticationFailed.

The temporary rev pin pointed at the feature branch, which has been
deleted on merge.  Pin to main so the plugin resolves the merge commit
that introduces PluginAuthenticationFailed.  This block can be removed
entirely once a new imbi-common release ships to PyPI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gmr gmr merged commit 5568863 into main May 9, 2026
3 checks passed
@gmr gmr deleted the feature/oidc-refresh-resilience branch May 9, 2026 22:40
gmr added a commit to AWeber-Imbi/imbi-api that referenced this pull request May 9, 2026
The dependent PRs have all merged:
- AWeber-Imbi/imbi-common#88 (a3475e2)
- AWeber-Imbi/imbi-plugin-aws#10 (dc4eab4)
- AWeber-Imbi/imbi-plugin-github#8 (5568863)

Restores rev = "main" pins for all three.  imbi-common does not
yet have a release that includes PluginAuthenticationFailed (latest
tag is v0.8.2); follow up with a tag bump once imbi-common cuts
v0.8.3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gmr added a commit to AWeber-Imbi/imbi-api that referenced this pull request May 9, 2026
## Summary

Three layers of defense against access tokens that expire between the
sweeper's last refresh and a user request.

### 1. Proactive refresh in `hydrate_identity`

`identity/resolution.py` — when the connection's `expires_at` is within
60 seconds of now AND a refresh token is present, refresh proactively
before materializing credentials. Smaller window than the sweeper's
5-minute lookahead so the two layers don't fight; this hook only catches
the race between sweeper ticks.

### 2. `call_with_identity_retry` helper

`identity/host_integration.py` — wraps a plugin call: on
`PluginAuthenticationFailed`, calls `flows.refresh_connection`,
re-hydrates ctx, and retries the closure once. A second 401 or a refresh
failure surfaces as the canonical `identity_required` 401 so the UI can
prompt the user to reconnect.

Pass `attached=True` when the caller has already invoked
`attach_identity` (e.g. a shared resolve helper that hydrates up-front
for read paths) to skip the redundant first hydration.

### 3. Endpoint retrofits

- `endpoints/project_configuration.py` — all 4 plugin call sites
(`list_keys`, `get_values`, `set_value`, `delete_key`)
- `endpoints/project_deployments.py` —
`_handle_deploy.trigger_deployment` and `_handle_promote.create_tag` /
`create_release` / `trigger_deployment` (4 sites). Each closure
recomputes credentials per attempt via `_resolve_credentials(ctx,
fallback)` so a refreshed identity's new access token is used on retry.

Read-only deployment endpoints
(refs/commits/compare/runs/promotion-options/draft-release-notes) still
use the existing `_resolve_and_context` path; they benefit from the
proactive refresh in `hydrate_identity` but skip the 401-retry overhead.

## Why

Found two production identity connections in `status='expired'` with
refresh tokens that AWS/GitHub were rejecting. Combined with the AWS
scope-tracking fix in imbi-plugin-aws#10, this should stop users from
being silently kicked out of OIDC connections mid-session.

End-to-end behavior now:
1. Steady state — sweeper refreshes 5 min before expiry every 60s.
2. Cold call near expiry — `hydrate_identity` proactively refreshes if
within 60s.
3. Mid-flight 401 on writes — retry helper refreshes and retries once.
4. AWS connections with stale cached client — auto-detected and
re-registered with `sso:account:access` on next connect.

## Dependencies

- AWeber-Imbi/imbi-common#88 — `PluginAuthenticationFailed` error
- AWeber-Imbi/imbi-plugin-aws#10 — AWS 401 detection + scope tracking
- AWeber-Imbi/imbi-plugin-github#8 — GitHub 401 detection

`pyproject.toml` is temporarily pinned to
`feature/oidc-refresh-resilience` of all three; **reset to released refs
(tag for imbi-common, main rev for plugins) before merging** (matches
the existing `Reset imbi-common + imbi-plugin-github to released refs`
pattern).

## Test plan

- [x] Three new tests in `tests/identity/test_resolution.py` cover
proactive refresh near expiry, no-refresh far from expiry,
refresh-failure-maps-to-401
- [x] Five new tests in `tests/identity/test_host_integration.py` cover
the retry helper end-to-end
- [x] Full test suite passes (935 endpoint + identity tests)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant