Add executor-specific config property bag to hooks with strongly-typed schema#7690
Merged
Conversation
jongio
approved these changes
Apr 13, 2026
jongio
left a comment
Member
There was a problem hiding this comment.
Clean plumbing PR. The Config map[string]any field follows the established ServiceConfig.Config pattern exactly, and the threading through �xecHook to ExecutionContext is surgical.
Two minor observations (neither blocking):
-
Signature calculation: json.Marshal is the right choice over manual key iteration since Config can contain nested maps. The silent error discard is fine in practice (YAML-sourced values are always JSON-compatible), but a brief inline comment would help future readers understand why the error is intentionally ignored.
-
Map by reference: Config is shared between HookConfig and ExecutionContext. Consistent with how ServiceConfig works, but worth a note in the ExecutionContext doc comment that executors shouldn't mutate it.
Test coverage is thorough - all value types, roundtrip, omitempty, signature inclusion, and end-to-end threading via the capture executor. Nice work.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an executor-specific Config map[string]any property bag to hook configuration and threads it through execution so hook executors can consume custom settings (core plumbing for #7653 / multi-language hook support).
Changes:
- Added
Config map[string]anytoHookConfig(YAMLconfig) and totools.ExecutionContext. - Threaded
HookConfig.ConfigintoExecutionContext.Configduring hook execution. - Added tests covering YAML parse/roundtrip and executor threading, plus signature inclusion.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/tools/script.go | Extends ExecutionContext with Config for executor-specific settings. |
| cli/azd/pkg/ext/models.go | Adds HookConfig.Config and includes it in hook signature calculation. |
| cli/azd/pkg/ext/hooks_runner.go | Threads hookConfig.Config into the execution context. |
| cli/azd/pkg/ext/models_test.go | Adds unit tests for config parsing/roundtrip/omitempty and signature inclusion. |
| cli/azd/pkg/ext/hooks_config_test.go | Adds hooks YAML unmarshal/marshal roundtrip tests including config blocks. |
| cli/azd/pkg/ext/hooks_runner_test.go | Adds integration-style test ensuring Config reaches the executor. |
This was referenced Apr 13, 2026
jongio
reviewed
Apr 13, 2026
jongio
left a comment
Member
There was a problem hiding this comment.
A few things I noticed on a second pass:
dotnet single-file mode silently ignores config - If a user sets configuration: Release or framework: net10.0 but there's no .csproj (single-file mode), those config values are silently dropped. The behavior is correct (single-file dotnet run doesn't support these flags), but a log warning when config is set but unused would save users from confusion.
wantEnsureSKip in python_executor_test.go - The struct field at line 685 is declared but never referenced in the test body. Also has a typo (SKip -> Skip).
Doc link in node_helpers.go:46 - The comment on validNodePackageManagers uses [project.packageManagerFromConfig] in Go doc link syntax, but that function is unexported in another package so the link won't resolve. Plain text reference would be cleaner.
Otherwise this looks great - clean layering, consistent use of UnmarshalHookConfig[T], solid test coverage.
Add a generic Config map[string]any field to HookConfig, following the established property bag pattern used by ServiceConfig, RemoteConfig, and platform.Config. The Config field is threaded through ExecutionContext so hook executors can access executor-specific settings. Changes: - Add Config field to HookConfig (yaml: config,omitempty) - Add Config field to ExecutionContext - Thread Config from HookConfig to ExecutionContext in execHook() - Include Config in hook configuration signature calculation - Add comprehensive tests for YAML parsing, roundtrip, omitempty, signature inclusion, and executor threading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement config integration for all hook executors, building on the Config property bag added to HookConfig and ExecutionContext. Shared infrastructure: - Add generic UnmarshalHookConfig[T] helper for type-safe config parsing from map[string]any via JSON re-marshal JS/TS executors (Azure#7692): - Add packageManager config (npm, pnpm, yarn) matching the existing ServiceConfig pattern from framework_service_node.go - Override auto-detection when config is specified - Shared validation in node_helpers.go used by both executors Python executor (Azure#7693): - Add virtualEnvName config to override default {baseName}_env naming - Path traversal validation rejects separators, dot components .NET executor (Azure#7694): - Add configuration config (Debug, Release, or custom MSBuild configs) - Add framework config for target framework moniker (net8.0, net10.0) - Flags applied in project mode only, single-file mode unchanged All changes include comprehensive table-driven tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Planning documents kept locally but removed from the PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add inline comment explaining intentional JSON marshal error handling in signature calculation - Update ExecutionContext.Config doc: nil-or-empty semantics and immutability contract - Add empty-map subtest to ConfigOmittedWhenEmpty test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover 6 additional scenarios for config block handling through the custom HooksConfig UnmarshalYAML: - Nested config maps (maps within maps) - Config with list/array values - Platform overrides (Windows/Posix) with config blocks - Type preservation (int, bool, float, string) through roundtrip - Empty config block (non-nil but empty map) - Config in list-format multi-hook definitions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update azure.yaml.json (v1.0 and alpha) with per-kind config validation using conditional allOf constraints on the kind field: - JS/TS hooks: config.packageManager (npm, pnpm, yarn) - Python hooks: config.virtualEnvName (string) - .NET hooks: config.configuration and config.framework (strings) Each config definition uses additionalProperties: false for strict validation. The generic config property is always allowed on hooks, with shape refined conditionally based on kind. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move config parsing inside project-mode branch in dotnet executor so single-file hooks are unaffected by config type errors - Fix comment reference: project.packageManagerFromConfig → nodePackageManagerFromConfig - Remove unused misspelled wantEnsureSKip test field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add emptyHookConfig definition with additionalProperties: false - Add conditional constraint for sh/pwsh kinds to reject arbitrary config keys - Add config: false to windows+posix mutual-override rule Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
847dd12 to
0292393
Compare
jongio
reviewed
Apr 14, 2026
jongio
left a comment
Member
There was a problem hiding this comment.
Solid work - the design follows the established ServiceConfig.Config pattern well, and the per-executor typed configs with schema validation are a clean approach. Two things I noticed that aren't covered by the existing feedback:
-
Python
virtualEnvNameconfig is bypassed when an existing venv is detected -detectExistingVenv(step 3) returns early before config parsing (step 4), so a user who addsvirtualEnvName: .venvafter already having amyproject_envwon't see the override take effect. Worth parsing config first or checking it inside the early-return path. -
Minor schema gap: the 'require run' rule (Rule 1 in both v1.0 and alpha) lists
interactive,continueOnError,secrets,shell,kind,dirin its anyOf but doesn't includeconfig. A hook with onlyconfig:passes schema validation but fails at runtime with ErrRunRequired.
- Reorder Prepare() so virtualEnvName config is parsed before detectExistingVenv, ensuring explicit config overrides win over auto-detected venvs - Add test verifying config override takes priority over existing .venv directory - Add config to schema require-run anyOf rule so hooks with only config: (no run:) fail validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
approved these changes
Apr 14, 2026
jongio
left a comment
Member
There was a problem hiding this comment.
All my prior feedback has been addressed. The Python config-ordering fix is verified correct - TestPythonPrepare_ConfigOverridesDetectedVenv confirms the config override wins over auto-detected venvs.
One low-priority note for a future follow-up: in single-file .NET mode, user-specified configuration/framework config values are silently ignored (correct behavior since dotnet run script.cs doesn't support those flags). A log.Printf warning when config is present but unused would help users catch misconfiguration early. Not blocking.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
rajeshkamal5050
approved these changes
Apr 14, 2026
rajeshkamal5050
added a commit
that referenced
this pull request
Apr 14, 2026
- Bump version from 1.23.16 to 1.24.0 (new features warrant minor bump) - Add #7290: azd init -t auto-creates project directory - Add #7690: executor-specific config block for hooks - Add #7703: fix PSModulePath conflict in azd update - Update version.txt and azdext/version.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rajeshkamal5050
added a commit
that referenced
this pull request
Apr 15, 2026
- Bump version from 1.23.16 to 1.24.0 (new features warrant minor bump) - Add #7290: azd init -t auto-creates project directory - Add #7690: executor-specific config block for hooks - Add #7703: fix PSModulePath conflict in azd update - Update version.txt and azdext/version.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
Hook definitions in
azure.yamlnow support an optionalconfig:block for passing executor-specific settings. Each hook kind (JS, TS, Python, .NET) accepts its own strongly-typed configuration properties:JS/TS
packageManagermatches the existingServiceConfig.Config["packageManager"]pattern used by framework services — users get the same override for hooks that they already have for services. PythonvirtualEnvNameaddresses the long-standing issue of the{baseName}_envdefault not matching the.venvconvention most projects use. .NETconfigurationandframeworkfill the gap where the hook executor defaulted to Debug while framework services hardcode Release.The
azure.yaml.jsonschema (v1.0 and alpha) validates config shapes per kind using conditionalallOfconstraints.How It Works
A generic
Config map[string]anyproperty bag is added toHookConfigand threaded throughExecutionContextto every executor — following the established pattern fromServiceConfig,RemoteConfig, andplatform.Config. A sharedUnmarshalHookConfig[T]generic helper converts the loosely-typed map into strongly-typed config structs via JSON re-marshal, providing nil-safety and clear type mismatch errors. Each executor unmarshals in itsPrepare()phase and validates executor-specific constraints (e.g., Python rejects path separators in venv names, JS/TS validates againstnpm/pnpm/yarn).The JSON schema uses the same
if/thenconditional pattern already used for host-specific service configs — whenkindis set, theconfigproperty is refined to the matching definition withadditionalProperties: falsefor strict validation and IDE autocomplete.Issue References
Resolves #7653
Resolves #7692
Resolves #7693
Resolves #7694
Resolves #7695
Relates to #7435
Notes
sh,pwsh) have no config properties yet — the config block is allowed but empty for these kinds