Resolve Pi's Databricks auth per request instead of baking a bearer - #532
Resolve Pi's Databricks auth per request instead of baking a bearer#532dhruv0811 wants to merge 2 commits into
Conversation
Pi's `models.json` carried the bearer as a literal `apiKey`, so a background
thread rewrote the file every 30 minutes to keep it from going stale. That
made `ucode` the required supervising parent process, wrote a live token to
disk, and still left the config holding a token up to 30 minutes old at
request time.
Pi has a native hook for this. A config value starting with `!` is executed
as a command and its stdout used as the value, and the provider auth path
resolves it through `resolveConfigValueOrThrow` (uncached) before every
provider request, not once per process. Verified against pi 0.84.3 with a
loopback gateway and a counting mint script: one turn that calls a tool
produces two provider requests and two distinct mints.
req1 auth=Bearer tok-1 stream=True tools=4
req2 auth=Bearer tok-2 stream=True tools=4
So each provider's `apiKey` becomes `!ucode auth-token --host ...`, and the
refresher thread goes away. Same shape OpenCode already moved to with its
auth plugin, and Pi joins Claude Code and Codex in resolving auth through
`ucode auth-token` rather than a value we have to keep fresh for it.
Notes:
- No `--force-refresh` (unlike OpenCode's plugin, which caches and needs it
for the 401 path). Pi has no cache here, so forcing a mint would round-trip
to the workspace on every turn; plain `auth-token` serves the CLI's cached
token until it nears expiry.
- `--profile` and `--use-pat` are forwarded, so PAT-configured workspaces keep
working via the same command.
- Pi does not re-mint and retry on a 401. The window shrinks from ~30 minutes
to the gap between mint and request, so it is no longer reachable in
practice, but it is worth knowing.
- `OAUTH_TOKEN` is still exported at launch and unchanged. Pi does not read it
for model auth (it ran fine without it in the probe), but removing it is a
separate question.
There was a problem hiding this comment.
🟡 Changes recommended
The Pi config backup path can still persist an old baked bearer token on disk, which undermines the security goal of avoiding tokens on disk.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Pi agent integration to avoid embedding Databricks bearer tokens in models.json by switching each provider’s apiKey to a Pi !command that runs ucode auth-token per request, eliminating the background token refresher thread and reducing token staleness and on-disk exposure.
Changes:
- Replace Pi providers’ literal bearer
apiKeywith a!ucode auth-token ...command viabuild_pi_api_key. - Remove the periodic token refresh thread from Pi launch, relying on per-request resolution instead.
- Update and extend tests to validate the new
!commandapiKey behavior and ensure tokens are not written tomodels.json.
File summaries
| File | Description |
|---|---|
| src/ucode/agents/pi.py | Switch Pi provider auth to a per-request !command apiKey and remove the token refresher thread. |
| tests/test_agent_pi.py | Update existing tests and add coverage for the new Pi apiKey command generation and “no bearer in models.json” behavior. |
| tests/test_e2e_user_agent.py | Adjust Pi e2e harness env so ucode auth-token can resolve via DATABRICKS_BEARER without a real workspace behind the capture server. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| backup_existing_file(PI_CONFIG_PATH, PI_BACKUP_PATH) | ||
| if token is None: | ||
| token = get_databricks_token( | ||
| state["workspace"], state.get("profile"), force_refresh=force_refresh | ||
| ) | ||
| token = get_databricks_token(state["workspace"], state.get("profile")) | ||
| pi_base_urls = state.get("base_urls", {}).get("pi") or build_pi_base_urls(state["workspace"]) |
The docstring claimed pi "never holds a token that can go stale". The bearer no longer lands in `models.json`, but `launch` still exports `OAUTH_TOKEN`, so a token does reach the process environment. State the real guarantee instead.
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently remove on-disk bearer persistence for Pi, eliminate the refresh thread, and add/adjust tests to cover the new on-demand auth-command behavior across providers.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Pi's
models.jsoncarried the bearer as a literalapiKey, so a background thread rewrote the file every 30 minutes to keep it from going stale (_refresh_forever,TOKEN_REFRESH_INTERVAL_SECONDS). Three costs:ucodehad to stay alive as the supervising parent, a live token sat on disk, and the config could still hold a 30-minute-old token at request time.Pi has a native hook for this. A config value starting with
!is run as a command and its stdout used as the value, and the provider auth path resolves it throughresolveConfigValueOrThrow(the uncached path) rather than the process-lifetime cache inresolve-config-value.js.So each provider's
apiKeybecomes!ucode auth-token --host ..., and the refresher goes away:providers["databricks-claude"] = { "baseUrl": pi_base_urls["claude"], "api": "anthropic-messages", - "apiKey": token, + "apiKey": api_key, # "!" + build_auth_shell_command(...) "authHeader": True,Net effect: Pi joins Claude Code and Codex in resolving auth through
ucode auth-token, and lands on the same shape OpenCode already moved to with its auth plugin. No token on disk, no supervisor requirement, no staleness window.Verified, not assumed
The load-bearing claim is "resolved per request, not once per process." Checked against pi 0.84.3 with a loopback fake gateway and an
apiKeyof!mint.sh, wheremint.shincrements a counter and printstok-N. Turn 1 returns atool_use, so pi runs the tool and issues a second provider request in the same process:Two requests, two distinct mints.
pi auth print-api-key --provider <name>independently confirms the!commandresolution.Notes
--force-refresh, unlike OpenCode's plugin. That plugin caches the token itself and needs the flag for its 401 retry; Pi has no cache on this path, so forcing a mint would round-trip to the workspace every turn. Plainauth-tokenserves the CLI's cached token until it nears expiry.--profileand--use-patare forwarded, so PAT-configured workspaces keep working through the same command.OAUTH_TOKENis unchanged, still exported at launch. Pi doesn't read it for model auth (the probe ran with only the!commandapiKey), so the launch-time mint may be droppable, but that's a separate question from this fix.Test plan
Updated:
test_token_in_api_key→test_api_key_config_value_embedded_verbatim:render_overlaymust embed the config value it's handed without reinterpreting it.test_config_written_with_correct_model_and_token→..._and_auth_command: the writtenapiKeyis a!-prefixedauth-tokencommand.tests/test_e2e_user_agent.py::TestPiUserAgent: setsDATABRICKS_BEARERin pi's env so the realucode auth-tokenthe config now invokes has something to print without a workspace behind the capture server. This makes the test stronger: it drives the real command end to end through the realpibinary, and it went from ~30s to ~1.5s because the request now arrives promptly.Added:
test_bearer_never_written_to_the_config: a real-looking token passed towrite_tool_configmust not appear anywhere inmodels.json.test_every_provider_gets_the_auth_command: all three providers, not just claude.TestBuildPiApiKey: leading!, runsauth-token, carries--host, omits--force-refresh, forwards--profileand--use-pat.Pre-existing failure, not from this change
tests/test_e2e_user_agent.py::TestClaudeUserAgent::test_user_agent_arrives_at_gatewayfails on cleanorigin/maintoo (verified by stashing this change and re-running:1 failed, 4 passed). Untouched here.Update: CI is green on both jobs, so that Claude failure is local to my machine (missing or mismatched
claudebinary), not a break onmain.Copilot review
"Docstring says pi never holds a token" — fair, the wording overclaimed. The bearer no longer lands in
models.json, butlaunchstill exportsOAUTH_TOKEN, so a token does reach the process environment. Docstring now states the real guarantee."The
models.jsonbackup can persist an old baked bearer" — real, but pre-existing and not pi-specific, so leaving it out of this PR.backup_existing_filereturns early when the backup already exists (config_io.py), so it captures the user's config once, on ucode's first write, and never refreshes. A ucode-written bearer only lands there on the narrow path where someone deletes the backup and re-runs an older ucode. The same mechanism backs up token-bearing configs for OpenCode, Gemini, Codex, and Claude today onmain, so the fix belongs inbackup_existing_file, covering all six, rather than as a pi-only patch here. Happy to send that separately.Verified in a live Kubernetes sandbox
Built this branch (merged with the
DATABRICKS_BEARER_COMMANDbranch) into a wheel, installed it in an agent-sandbox Pod launched by an Omnigent server, and configured all four harnesses with no credential in the Pod other than a bearer command.Pi's
models.json, written by this branch:A command on every provider, no
--force-refresh, and grepping the actual bearer against every file ucode wrote under its app dir found it in none of Pi's.ucode configure --agents claude,codex,pi,opencodethen reported Pi is working from its live validation message, so a real turn went through Databricks AI Gateway resolving auth per request.Worth noting for contrast, since it is the same problem this PR fixes and is unchanged on
main: OpenCode'sopencode.jsonstill carries the bearer as a literalapiKey(a 791-char token on disk, for all three providers). Its plugin mints on demand whenaccessTokenis unset, so that seed looks like an optimization paid for with a token on disk. Separate change; noting it here only because Pi and OpenCode were the two baked-bearer harnesses and only one of them is fixed by this PR.