didericis-claude
e528d5c5af
Revises the Design section to describe the implemented solution: provisioned_env on AgentProvisionPlan rather than an intermediate egress_resolve_token_values_with_provider function. Drops the old sentinel/lazy-import design narrative.
122 lines
4.8 KiB
Markdown
122 lines
4.8 KiB
Markdown
# PRD 0030: Deduplicate egress token resolution across backends
|
|
|
|
- **Status:** Active
|
|
- **Author:** didericis-claude
|
|
- **Created:** 2026-06-02
|
|
- **Issue:** #118
|
|
|
|
## Summary
|
|
|
|
Eliminate the duplicated egress token resolution block — which resolved
|
|
manifest-declared tokens and the Codex host credential — by moving
|
|
provider-specific token reading into `AgentProvisionPlan.provisioned_env`
|
|
at prepare time, and having both backends merge that map into `os.environ`
|
|
before calling the now-generic `egress_resolve_token_values`.
|
|
|
|
## Problem
|
|
|
|
The same logic block appeared in two places:
|
|
|
|
- `bot_bottle/backend/docker/launch.py` (~line 183): inline inside
|
|
`launch()`, building `token_values` before `compose_up`.
|
|
- `bot_bottle/backend/smolmachines/launch.py` (~line 422): the private
|
|
`_resolve_token_env` helper, called before `_bundle.start_bundle`.
|
|
|
|
Both blocks:
|
|
1. Short-circuit to `{}` when there are no egress routes.
|
|
2. Call `egress_resolve_token_values(token_env_map, host_env)` to
|
|
resolve ordinary manifest-declared token refs.
|
|
3. Check `agent_provider.forward_host_credentials` and, when true,
|
|
call `codex_host_access_token` and slot the result into any
|
|
`token_env` whose `token_ref` is `CODEX_HOST_CREDENTIAL_TOKEN_REF`.
|
|
|
|
The duplication means any change to step 3 must be applied twice.
|
|
PRD 0029, which introduced `forward_host_credentials`, already had to
|
|
wire both backends; the next change would too. This is a near-certain
|
|
future sync bug.
|
|
|
|
`egress_resolve_token_values` also carried a sentinel `continue` skip
|
|
for `CODEX_HOST_CREDENTIAL_TOKEN_REF`, which tied an otherwise generic
|
|
egress helper to a Codex-specific contract.
|
|
|
|
## Goals / Success Criteria
|
|
|
|
- The `forward_host_credentials` resolution logic exists in exactly one
|
|
place in the codebase.
|
|
- Both `docker/launch.py` and `smolmachines/launch.py` call
|
|
`egress_resolve_token_values` with no provider-specific arguments.
|
|
- `egress_resolve_token_values` is fully generic — it neither knows nor
|
|
cares about provider identity or the `CODEX_HOST_CREDENTIAL_TOKEN_REF`
|
|
sentinel.
|
|
- No behaviour change for either backend.
|
|
|
|
## Non-goals
|
|
|
|
- Changes to token resolution semantics. This is a pure refactor.
|
|
- Adding support for any new credential type or provider.
|
|
- Consolidating any other backend differences beyond this one block.
|
|
|
|
## Design
|
|
|
|
### `AgentProvisionPlan.provisioned_env`
|
|
|
|
Add `provisioned_env: dict[str, str]` (default empty) to
|
|
`AgentProvisionPlan`. This map holds host-side secrets that the
|
|
provisioning stage resolved and that egress needs injected into the
|
|
sidecar environ.
|
|
|
|
When `forward_host_credentials=True` for Codex, `agent_provision_plan`
|
|
calls `codex_host_access_token(host_env)` and stores the result under
|
|
`CODEX_HOST_CREDENTIAL_TOKEN_REF`. This is already the prepare-time
|
|
stage where `write_codex_dummy_auth_file` runs, so the access-token
|
|
read is colocated with all other Codex-specific provisioning.
|
|
|
|
### Backend call sites
|
|
|
|
Both backends merge `provisioned_env` over `os.environ` before calling
|
|
`egress_resolve_token_values`:
|
|
|
|
```python
|
|
effective_env = {**dict(os.environ), **plan.agent_provision.provisioned_env}
|
|
token_values = egress_resolve_token_values(plan.egress_plan.token_env_map, effective_env)
|
|
```
|
|
|
|
`provisioned_env` values take precedence over same-named host env vars,
|
|
so the Codex token slot resolves from the map written at prepare time
|
|
rather than from a raw `os.environ` lookup.
|
|
|
|
### `egress_resolve_token_values`
|
|
|
|
Remove the `CODEX_HOST_CREDENTIAL_TOKEN_REF` sentinel `continue` skip.
|
|
The function now resolves every slot in `token_env_map` from `host_env`
|
|
without special-casing any key name. The `CODEX_HOST_CREDENTIAL_TOKEN_REF`
|
|
key is present in `effective_env` (injected by `provisioned_env`) exactly
|
|
when it is needed.
|
|
|
|
## Implementation
|
|
|
|
- `bot_bottle/agent_provider.py`: add `provisioned_env` field; populate
|
|
it for Codex when `forward_host_credentials=True`.
|
|
- `bot_bottle/egress.py`: remove sentinel skip; remove
|
|
`egress_resolve_token_values_with_provider` (the intermediate function
|
|
that was introduced and then superseded by this design); drop the
|
|
`codex_auth` import.
|
|
- `bot_bottle/backend/docker/launch.py`: replace the provider-aware
|
|
resolution block with the `effective_env` merge.
|
|
- `bot_bottle/backend/smolmachines/launch.py`: same replacement in
|
|
`_resolve_token_env`.
|
|
- `tests/unit/test_agent_provider.py`: add tests verifying
|
|
`provisioned_env` is populated (or empty) for Codex with and without
|
|
`forward_host_credentials`.
|
|
- `tests/unit/test_egress.py`: remove `TestResolveTokenValuesWithProvider`;
|
|
replace the sentinel-skip test with a test verifying the Codex token
|
|
ref resolves normally when present in `host_env`.
|
|
|
|
## References
|
|
|
|
- Issue #118: Deduplicate egress token resolution across backends.
|
|
- Issue #117: Complexity hotspots in launch, egress, and auth paths
|
|
(source of the finding).
|
|
- PRD 0029: Provider auth credentials through egress (introduced the
|
|
duplicated block).
|