Skip to content

Force token refresh in the auth-token apiKeyHelper#191

Open
brodrigu wants to merge 5 commits into
databricks:mainfrom
brodrigu:fix/claude-token-force-refresh
Open

Force token refresh in the auth-token apiKeyHelper#191
brodrigu wants to merge 5 commits into
databricks:mainfrom
brodrigu:fix/claude-token-force-refresh

Conversation

@brodrigu

@brodrigu brodrigu commented Jul 8, 2026

Copy link
Copy Markdown

Problem

Long-running Claude Code / Codex sessions die when the original Databricks OAuth token expires (~1h), even though the apiKeyHelper is configured to refresh every 15 min via CLAUDE_CODE_API_KEY_HELPER_TTL_MS: 900000.

Root cause

ucode auth-token is the command Claude Code's apiKeyHelper (and Codex's auth command) re-invokes on that 15-min interval. It called get_databricks_token(...) without force_refresh, so the underlying databricks auth token only renewed the token when the CLI independently judged the cached token "close to expiry":

Get authentication token from the local cache… Refresh the access token if it is expired or close to expiry. Use --force-refresh to bypass expiry checks.

That heuristic can lapse between helper invocations, so the session dies at the original token's expiry. Notably, this is inconsistent with the gemini/opencode/copilot/pi agents, whose background refreshers already pass force_refresh=True.

Fix

Pass force_refresh=True from the auth-token helper so each refresh cycle bypasses the expiry heuristic and hands back a token with a full fresh lifetime.

Tests

  • tests/test_cli.py — the auth-token command forwards force_refresh=True.
  • tests/test_databricks.py--force-refresh is (and by default isn't) forwarded to the real databricks auth token argv.

Full suite passes (853 passed); the only failures are two pre-existing network-dependent e2e tests that also fail on clean main.

Note for reviewers

PR #137 (Windows support) touches this same auth code and is stale against #177 — the two will conflict.

🤖 Generated with Claude Code

`ucode auth-token` is the command Claude Code's apiKeyHelper (and Codex's
auth command) re-invokes every 15 min via CLAUDE_CODE_API_KEY_HELPER_TTL_MS.
It called `databricks auth token` without --force-refresh, so the token was
only renewed when the CLI independently judged the cached token "close to
expiry" — which can lapse between invocations, killing a long session at the
original token's ~1h expiry.

Pass force_refresh=True so each refresh cycle hands back a token with a full
fresh lifetime, matching the force_refresh=True already used by the
gemini/opencode/copilot/pi background refreshers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 15:52

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

This PR fixes long-running agent sessions (Claude Code/Codex) failing at ~1 hour due to Databricks OAuth token expiry by forcing a refresh on every ucode auth-token helper invocation, bypassing the Databricks CLI’s “close to expiry” heuristic.

Changes:

  • Update ucode auth-token to call get_databricks_token(..., force_refresh=True) every time.
  • Add CLI-level test coverage ensuring auth-token forwards force_refresh=True.
  • Add Databricks argv-forwarding tests verifying --force-refresh is appended only when requested.

Reviewed changes

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

File Description
src/ucode/cli.py Forces refresh in auth-token helper to prevent token-expiry session drops.
tests/test_cli.py Verifies auth-token forwards force_refresh=True and keeps stdout clean.
tests/test_databricks.py Verifies get_databricks_token forwards --force-refresh appropriately.

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

Comment thread tests/test_cli.py
Comment thread tests/test_cli.py
Comment thread tests/test_cli.py
Mirror the real get_databricks_token signature (force_refresh is
keyword-only) in the --use-pat test lambdas, so the test would fail if the
production call site ever passed force_refresh positionally. Addresses PR
review feedback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 15:56
@brodrigu

brodrigu commented Jul 8, 2026

Copy link
Copy Markdown
Author

Thanks — good catch. Addressed in 08bd1d4: the three --use-pat test mocks now declare force_refresh as keyword-only (lambda w, p, *, force_refresh=False), mirroring the real get_databricks_token signature so the tests fail if the call site ever passes it positionally.

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 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread tests/test_databricks.py Outdated
Comment thread tests/test_databricks.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 16:00

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 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread tests/test_databricks.py
Comment thread tests/test_databricks.py
…spaces

- Strip DATABRICKS_BEARER in the _fake_databricks helper: get_databricks_token
  short-circuits on it, so a leaked CI/developer bearer would bypass the fake
  CLI shim and make the whole TestGetDatabricksToken class's assertions
  meaningless.
- Quote the argv-log redirection target in the fake shim so tests don't break
  when the temp path contains spaces.

Addresses PR review feedback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 16:22
@brodrigu

brodrigu commented Jul 8, 2026

Copy link
Copy Markdown
Author

Pushed 54b4432 on top of the accepted quoting suggestion:

  • Path quoting — extended the >> "$argv_log" quoting to the third (pre-existing) occurrence in test_passes_profile_flag_when_provided so the fake shim is consistent.
  • DATABRICKS_BEARER short-circuit — the remaining Copilot concern. Rather than patching each test, I strip DATABRICKS_BEARER once in the shared _fake_databricks helper, since get_databricks_token short-circuits on it and every test in TestGetDatabricksToken needs the fake CLI shim to actually run. A leaked CI/developer bearer would otherwise silently bypass the shim.

All 216 tests in test_databricks.py + test_cli.py pass; lint/format clean.

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 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 8, 2026 16:34

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 3 out of 3 changed files in this pull request and generated no new comments.

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.

2 participants