Skip to content

feat(config): accept ${env.X} in toolset env values (#2615)#3257

Merged
dgageot merged 2 commits into
docker:mainfrom
dgageot:worktree-board-d3aa9ed4a50f4f56
Jun 26, 2026
Merged

feat(config): accept ${env.X} in toolset env values (#2615)#3257
dgageot merged 2 commits into
docker:mainfrom
dgageot:worktree-board-d3aa9ed4a50f4f56

Conversation

@dgageot

@dgageot dgageot commented Jun 26, 2026

Copy link
Copy Markdown
Member

Toolset env values previously only understood the shell-style ${X} expansion syntax. When an agent config used the JS-template form ${env.X} — which already works in path fields like working_dir and path — the runtime would reject it with a hard "environment variable not set" error, silently breaking configs that mixed both styles. This is the first part of fixing the inconsistency tracked in #2615.

The fix extracts a shared NormalizeEnvRefs helper in pkg/path/expand.go that rewrites ${env.X} to ${X} before expansion, then calls it from pkg/environment/expand.go ahead of os.Expand. Richer JS expressions such as ${env.VAR || 'default'} are intentionally not evaluated in env values — only the plain ${env.X} alias is handled. The variable-expansion quick-reference table in docs/configuration/overview/index.md has been updated to reflect that env values now accept both forms.

A related cleanup drops the now-obsolete startup warning that flagged ${env.X} in toolset env values. The warning helper for shell and hook fields — which are forwarded verbatim to exec.Cmd with no expansion at all — was renamed warnPathFieldwarnExecField and its message corrected; the old message incorrectly told users to switch to ${X} or $X, neither of which is substituted in those fields.

@dgageot dgageot requested a review from a team as a code owner June 26, 2026 09:45

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The PR correctly implements NormalizeEnvRefs to rewrite ${env.X}${X} before os.Expand, and integrates it cleanly in both pkg/path/expand.go and pkg/environment/expand.go. The regex replacement string ${\} is valid Go regexp syntax and produces the expected output. The warning removal and warnPathFieldwarnExecField rename are consistent with the change. No bugs introduced by this PR were found.

@aheritier aheritier added area/config For configuration parsing, YAML, environment variables area/docs Documentation changes kind/feat PR adds a new feature (maps to feat:). Use on PRs only. labels Jun 26, 2026
@dgageot dgageot merged commit edc6be0 into docker:main Jun 26, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config For configuration parsing, YAML, environment variables area/docs Documentation changes kind/feat PR adds a new feature (maps to feat:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants