fix(backend,runner): track GitHub App token expiry for proactive refresh#889
Merged
Gkrumbach07 merged 4 commits intomainfrom Mar 13, 2026
Merged
fix(backend,runner): track GitHub App token expiry for proactive refresh#889Gkrumbach07 merged 4 commits intomainfrom
Gkrumbach07 merged 4 commits intomainfrom
Conversation
GitHub App installation tokens expire after ~1 hour but the runner had no expiry tracking. The backend now returns expiresAt in the credential response, and the runner proactively refreshes tokens before they expire (5 min buffer). Backend changes: - GetGitHubToken returns (token, expiresAt, error) instead of (token, error) - runtime_credentials.go includes expiresAt in GitHub credential response - WrapGitHubTokenForRepo updated for new signature Runner changes: - auth.py tracks GitHub token expiry from backend response - bridge.py checks token expiry in _refresh_credentials_if_stale() Fixes: RHOAIENG-52858 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough
Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as HTTP Handler
participant GitOps as Git Operations
participant KubeSecret as Kubernetes Secret
participant Runner as Ambient Runner
participant Auth as Auth Module
rect rgba(100,150,200,0.5)
Note over Client,Handler: Token retrieval with expiry
Client->>Handler: Request GitHub credentials
Handler->>GitOps: GetGitHubToken(ctx,...)
GitOps->>KubeSecret: Fetch token & metadata
KubeSecret-->>GitOps: Return token [+ expiresAt]
GitOps-->>Handler: (token, expiresAt, error)
Handler->>Client: Response includes expiresAt if non-zero
end
rect rgba(150,100,200,0.5)
Note over Runner,Auth: Runner refresh decision
Runner->>Auth: github_token_expiring_soon()?
alt expiring soon
Auth-->>Runner: true
else not expiring
Runner->>Runner: check interval since last refresh
end
opt Refresh needed
Runner->>Handler: Fetch fresh credentials
Handler->>GitOps: GetGitHubToken(...)
GitOps-->>Handler: (token, expiresAt, error)
Handler-->>Runner: credentials with expiresAt
Runner->>Auth: update cached token & expiry
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/runners/ambient-runner/ambient_runner/bridge.py`:
- Around line 58-65: The done-callback for the shutdown task calls f.exception()
even when the Future was cancelled, which raises CancelledError; update the
callback attached in task.add_done_callback (the lambda receiving f) to first
check f.cancelled() (or use f.done() and not f.cancelled()) and only call
f.exception() when not cancelled, or wrap the call in a try/except and log
exceptions only when present and not a cancellation; ensure the callback uses
_bridge_logger.warning with the exception details only for real exceptions and
ignores cancelled tasks.
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py`:
- Around line 146-152: The current datetime.fromisoformat call in the GitHub
token expiry block fails on RFC3339 strings that end with a trailing "Z" (Python
3.10); update the parsing in the block that sets _credential_expiry["github"] so
it normalizes a trailing "Z" to "+00:00" (or otherwise convert "Z" to an
RFC3339-compatible offset) before calling datetime.fromisoformat, then set
_credential_expiry["github"] = exp_dt.timestamp() and keep the existing
logger.info/logger.warning behavior (keep catching ValueError/TypeError on parse
and logging the failure with the exception).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ca148f7-fb64-4add-8f47-49da84e11051
📒 Files selected for processing (6)
components/backend/git/operations.gocomponents/backend/handlers/github_auth.gocomponents/backend/handlers/operations_test.gocomponents/backend/handlers/runtime_credentials.gocomponents/runners/ambient-runner/ambient_runner/bridge.pycomponents/runners/ambient-runner/ambient_runner/platform/auth.py
Remove the 3-minute GitHub App token cache so refresh_credentials always mints fresh tokens. Fix restart_session tool crash by passing bridge instance instead of None for deferred adapter access. Fixes: RHOAIENG-52858 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py`:
- Around line 33-38: The build_mcp_servers function currently accepts
bridge_ref: Any = None but restart_session (registered inside build_mcp_servers)
requires a non-null bridge_ref, so callers can silently register a broken tool;
fix by making bridge_ref required or fail fast: either change the signature of
build_mcp_servers to require bridge_ref as a keyword-only parameter (remove the
None default) or add an immediate check at the start of build_mcp_servers that
raises a clear ValueError if bridge_ref is None; reference the build_mcp_servers
function and the restart_session registration to locate where to apply this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5bcc647-799d-4c71-baca-9e75d5f4fe75
📒 Files selected for processing (4)
components/backend/github/token.gocomponents/runners/ambient-runner/ambient_runner/bridges/claude/bridge.pycomponents/runners/ambient-runner/ambient_runner/bridges/claude/mcp.pycomponents/runners/ambient-runner/ambient_runner/bridges/claude/tools.py
💤 Files with no reviewable changes (1)
- components/backend/github/token.go
- Fix CancelledError when calling exception() on cancelled futures in shutdown callback - Fix Python 3.10 RFC3339 parsing by replacing Z with +00:00 timezone offset - Clear stale GitHub token expiry on parse failures - Auto-format code per pre-commit hooks Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
🤖 PR Fixer ReportSummaryAddressed CodeRabbit review feedback by fixing two critical bugs affecting Python 3.10 compatibility and asyncio exception handling. What Was Fixed1. CancelledError in asyncio shutdown callback (bridge.py:58-65)
2. Python 3.10 RFC3339 timestamp parsing (auth.py:148)
What Was Skipped3. Making bridge_ref required in mcp.py (mcp.py:38)
Code ReviewRan
CI Status
Commits Pushed
All reviewer feedback has been addressed. The PR is ready for merge. |
The restart_session tool crashed the CLI with exit code 1 because setting _restart_requested on the adapter mid-stream caused an unrecoverable error. Remove the tool entirely until a proper deferred restart mechanism is implemented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Fixes RHOAIENG-52858: GitHub App installation tokens expire after ~1 hour, causing silent auth failures in long-running sessions.
Changes
Backend (
components/backend/)GetGitHubToken()ingit/operations.goto return(string, time.Time, error), propagating theexpiresAtfromMintInstallationTokenForHostGetGitHubTokenForSessionhandler to includeexpiresAtin the JSON response when non-zeroWrapGitHubTokenForRepowrapper to accept the new 3-return signature and discardexpiresAtRunner (
components/runners/ambient-runner/)_credential_expirytracking dict and_EXPIRY_BUFFER_SEC(5 min) constant inauth.pyfetch_github_credentials()now parsesexpiresAtfrom backend response and stores expiry timestampgithub_token_expiring_soon()helper for proactive refresh checks_refresh_credentials_if_stale()inbridge.pynow also triggers refresh when GitHub token is expiring soonHow it works
expiresAt(RFC 3339) for GitHub App tokens in the credential response_credential_expiry["github"]run(), the bridge checks both the 60s refresh interval AND token expiry (with 5-min buffer)PAT tokens (which don't expire) are unaffected — they return no
expiresAtand skip expiry tracking.Test plan
cd components/backend && go build ./...)Fixes: RHOAIENG-52858
Jira: RHOAIENG-52858