Skip to content

models-config: apply config env vars before implicit provider discovery#32295

Merged
steipete merged 2 commits intoopenclaw:mainfrom
hsiaoa:fix/bedrock-discovery-config-env-vars
Mar 3, 2026
Merged

models-config: apply config env vars before implicit provider discovery#32295
steipete merged 2 commits intoopenclaw:mainfrom
hsiaoa:fix/bedrock-discovery-config-env-vars

Conversation

@hsiaoa
Copy link
Contributor

@hsiaoa hsiaoa commented Mar 2, 2026

Summary

Fixes #32290.

ensureOpenClawModelsJson() calls resolveImplicitBedrockProvider() which checks for AWS credentials in process.env (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID). However, some callers (agent runner, tools, compaction) pass config objects that haven't gone through the full loadConfig() pipeline — meaning config-defined env vars from openclaw.json env.vars are never applied to process.env before discovery runs.

This causes Bedrock discovery to silently fail or skip when AWS credentials are only configured via openclaw.json env.vars rather than being present in the host shell environment.

The fix calls applyConfigEnvVars(cfg) at the top of ensureOpenClawModelsJson before any implicit provider discovery. This is idempotent (only sets vars not already present in process.env) and matches the existing pattern used in loadConfig().

Test plan

  • All existing models-config tests pass (68/68)
  • All config.env-vars tests pass (6/6)
  • Build succeeds with no type errors
  • Lint/format clean
  • Manual: configure AWS_PROFILE only in openclaw.json env.vars, verify openclaw status shows Bedrock models discovered

@openclaw-barnacle openclaw-barnacle bot added agents Agent runtime and tooling size: XS labels Mar 2, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Greptile Summary

This PR fixes a bug where Bedrock (and other implicit provider) discovery silently failed when AWS credentials were configured via openclaw.json env.vars rather than the host shell environment. The root cause was that ensureOpenClawModelsJson could be called by agent runners, tools, and compaction with a config object that hadn't passed through the full loadConfig() pipeline, so applyConfigEnvVars was never called and process.env was never populated with config-defined vars before resolveImplicitProviders / resolveImplicitBedrockProvider ran.

Changes:

  • Adds applyConfigEnvVars(cfg) at the top of ensureOpenClawModelsJson, before the implicit provider discovery calls.
  • The fix is idempotent — applyConfigEnvVars is a no-op for vars already present in process.env — so there is no risk of double-application when called from a code path that already went through loadConfig().
  • No new test covering the specific scenario (config-only AWS credentials triggering Bedrock discovery) is included; the manual test step in the PR checklist is still unchecked, but the change is simple and the existing test suite validates surrounding logic.

Confidence Score: 5/5

  • This PR is safe to merge — the change is minimal, targeted, and idempotent with no risk of regressions.
  • The fix is a single, well-understood function call (applyConfigEnvVars) that is already used in the same pattern within loadConfig(). The function is idempotent by design (skips vars already set in process.env), so calling it on code paths that already went through loadConfig() is harmless. All 68 existing models-config tests and 6 config.env-vars tests pass. The only gap is that no automated test covers the exact regression scenario (config-only AWS credentials → Bedrock discovery), but the change is simple enough that the risk is very low.
  • No files require special attention.

Last reviewed commit: 1a51276

@steipete steipete force-pushed the fix/bedrock-discovery-config-env-vars branch from 1a51276 to b53b0c1 Compare March 3, 2026 00:25
@steipete steipete merged commit 1e8afa1 into openclaw:main Mar 3, 2026
@steipete
Copy link
Contributor

steipete commented Mar 3, 2026

Landed via temp rebase onto main.

  • Gate: pnpm test -- src/agents/models-config.applies-config-env-vars.test.ts src/agents/models-config.skips-writing-models-json-no-env-token.test.ts src/agents/models-config.auto-injects-github-copilot-provider-token-is.test.ts
  • Note: full pnpm check is currently red on unrelated main-branch failures in extensions/tlon and src/discord, src/media-understanding, src/security type checks.
  • Land commit: b53b0c1
  • Merge commit: 1e8afa1

Thanks @hsiaoa!

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b53b0c1799

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// available in process.env before implicit provider discovery. Some
// callers (agent runner, tools) pass config objects that haven't gone
// through the full loadConfig() pipeline which applies these.
applyConfigEnvVars(cfg);

Choose a reason for hiding this comment

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

P2 Badge Apply config env vars before resolving agentDir

Calling applyConfigEnvVars(cfg) after agentDir is computed means configs that set env.vars.OPENCLAW_AGENT_DIR (or PI_CODING_AGENT_DIR) are applied too late: ensureOpenClawModelsJson writes models.json to the pre-env directory, then mutates process.env to a different directory. In flows that re-resolve the agent dir after this call (for example loadModelCatalog in src/agents/model-catalog.ts), discovery can read from a different path than the one just written, so configured/implicit models may appear missing when the config object is passed directly (not via loadConfig()).

Useful? React with 👍 / 👎.

planfit-alan pushed a commit to planfit/openclaw that referenced this pull request Mar 3, 2026
dawi369 pushed a commit to dawi369/davis that referenced this pull request Mar 3, 2026
OWALabuy pushed a commit to kcinzgg/openclaw that referenced this pull request Mar 4, 2026
AytuncYildizli pushed a commit to AytuncYildizli/openclaw that referenced this pull request Mar 4, 2026
zooqueen pushed a commit to hanzoai/bot that referenced this pull request Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bedrock-discovery ignores env.vars and AWS credentials from config

2 participants