feat(agents): add environment support to ExecuteAgentJob - #1486
Merged
Conversation
Give ExecuteAgentJob the same AgentEnvironment support that PR #1379 added to AgentDeployment. An execute job can now reference an AgentEnvironment (ref or inline); its EnvironmentSpec is merged into the agent config and its ComputeSpec and secret-env references are snapshotted onto the job step at creation time. - ExecuteAgentJobConfig gains an 'environment' input field. - ExecuteAgentStepConfig snapshots environment (raw), compute, and secrets. - to_spec resolves + merges the environment (reusing environment_resolution) and validates the merged config; EnvironmentResolutionError surfaces as a ValueError on create. - compile injects each secret ref as a secret-backed step env var (guarded against reserved job/agent env names) and maps ComputeResources onto the executor ResourcesSpec (cpu/memory pass through, nvidia.com/gpu -> num_gpus, unsupported keys rejected). Regenerated the nemo-agents plugin OpenAPI spec. Signed-off-by: Ben McCown <bmccown@nvidia.com>
Contributor
|
The jobs step 'executor' is a union of executor TypedDicts, and SubprocessExecutionProviderParam has no 'resources' key, so ty's full-repo check (lint-python-types) flagged step["executor"]["resources"] as an invalid key. Cast the executor to dict[str, Any] before subscripting, matching the existing step["config"] pattern in this file. Signed-off-by: Ben McCown <bmccown@nvidia.com>
The reserved-name secret-env collision and unsupported-compute-resource-key checks live in ExecuteAgentJob.compile and raised a bare ValueError. The jobs create route's compile wrapper (_compile_platform_spec) only translates PlatformJobCompilationError into a 422 — a bare ValueError escaped to the global handler as an opaque 500 "An unexpected error occurred", unlike the to_spec validations (missing env ref, non-local provider) which surface as a descriptive 422. Wrap the two compile-time snapshot validations and re-raise their ValueError as PlatformJobCompilationError so both reach the client as a 422 with the original message. Update the two compile unit tests to expect the new type and add a route-level test asserting the reserved-name collision maps to 422 at the HTTP boundary (the existing unit test only checked the raised exception, not the mapped status code). Signed-off-by: Ben McCown <bmccown@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughExecute jobs now accept stored or inline environments. Environment, compute, and secret settings are resolved, snapshotted, compiled into platform steps, and validated through unit and HTTP tests. ChangesExecute job environment support
Sequence Diagram(s)sequenceDiagram
participant ExecuteJob
participant EntityClient
participant Executor
participant PlatformStep
ExecuteJob->>EntityClient: Resolve environment and compute settings
EntityClient-->>ExecuteJob: Return environment and secrets
ExecuteJob->>ExecuteJob: Merge configuration and snapshot settings
ExecuteJob->>Executor: Compile compute resources
ExecuteJob->>PlatformStep: Pass executor and environment variables
PlatformStep-->>ExecuteJob: Return compiled step
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change adds environment support to execute jobs with validation and resource/secret propagation, and the supplied checks pass; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
mikeknep
reviewed
Aug 25, 2026
…tepConfig ExecuteAgentStepConfig stored a top-level environment field that to_spec set to exactly request.environment. Since the step config already persists the whole request: ExecuteAgentJobConfig, that field carried no information not already in request.environment — the raw-request provenance is already there. Drop the redundant field and read provenance off request.environment instead. compute/secrets stay: those are resolved snapshots that do not exist on the raw request. Unlike AgentDeployment (which keeps a top-level environment because the entity does not embed the raw request), the job step config already embeds it, so the field is genuinely redundant here. Updates the two to_spec tests to assert provenance via request.environment, and regenerates the nemo-agents OpenAPI spec (diff scoped to the removed field). Signed-off-by: Ben McCown <bmccown@nvidia.com>
mikeknep
approved these changes
Aug 25, 2026
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
2 participants
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.
Summary
Gives
ExecuteAgentJobthe sameAgentEnvironmentsupport that PR #1379 added toAgentDeployment. An execute job can now reference anAgentEnvironment(a"workspace/name"ref or inline); at create time itsEnvironmentSpecis merged into the resolved agent config and itsComputeSpecand secret-env references are snapshotted onto the job step. Before, an execute job ran only the bare agent config with no environment dependencies, resources, or secret-backed env vars; after, the run gets the merged config, executor resources from compute, and secret-backed step env vars.Related Issue
Changes
ExecuteAgentJobConfig: newenvironment: str | AgentEnvironmentInline | Noneinput field.ExecuteAgentStepConfig: new snapshot fieldsenvironment(raw request value),compute: ComputeSpecInline | None, andsecrets: dict[str, str], mirroringAgentDeployment.to_spec: resolves and merges the referenced environment (reusing the sharedenvironment_resolutionhelpers), snapshots compute/secrets/raw environment, and validates the merged config so anEnvironmentSpecselecting a non-local provider is rejected.EnvironmentResolutionErroris surfaced as aValueErroron the create path (matching the existing "Agent not found" pattern).compile: injects each secret ref as a secret-backed step env var viaEnvironmentVariable(from_secret=...), guarded against a reserved set (theNEMO_JOB_*/NMP_TASK_CONFIGjob-substrate vars plus the deployment-container reserved names), and maps agentsComputeResourcesonto the executorResourcesSpec(cpu/memorypass through;nvidia.com/gpu->num_gpus; any other resource key is rejected).run: unchanged — Fabric reads the merged config and inherits the substrate-populated process env (MCP secrets resolved via env-var-name indirection).test_execute_job.py(environment ref/inline snapshot, no-environment empty snapshot, missing-ref -> ValueError, non-local-provider rejection, MCP secret indirection, compile secret env + compute resources, no-compute omits executor resources, unsupported resource key raises, reserved-name collision raises). Also refreshed a pre-existing stale snapshot assertion in the create-route test (the Fabricenvironmentblock gainedenv/connection/metadatadefaults in feat(agents): add AgentEnvironment / EnvironmentSpec / ComputeSpec entities #1379; that assertion failed onmainindependently of this change).make refresh-openapi); the diff is scoped to the three new fields (AgentEnvironmentInline/ComputeSpecInlineschemas already existed). The nemo-agents plugin is not part of Stainless SDK generation, so no SDK regen.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
uv run ruff check plugins/nemo-agents/src/nemo_agents_plugin/jobs/execute.py plugins/nemo-agents/tests/unit/test_execute_job.py— passed.uv run ruff format --check(same files) — passed (already formatted).uv run --frozen ty checkon changed source + test files — passed.bash tools/lint/lint-python-types.sh(full-repoty check, mirrors CI'slint-python-types) — passed. (Initial CI run flaggedstep["executor"]["resources"]in a compile test: the stepexecutoris a union of executor TypedDicts andSubprocessExecutionProviderParamhas noresourceskey. Fixed by casting the executor todict[str, Any]before subscripting, matching the existingstep["config"]pattern.)uv run --frozen pytest plugins/nemo-agents/tests/unit/test_execute_job.py --import-mode=importlib -p no:cacheprovider -q— 50 passed.uv run --frozen pytest plugins/nemo-agents/tests/unit --import-mode=importlib -p no:cacheprovider -q— 1335 passed (no regressions).make refresh-openapi— completed; diff scoped toplugins/nemo-agents/openapi/openapi.yaml.uv run pre-commit run -a— relevant hooks passed (ruff, ruff format, ty, config-reference, copyright headers, plugins-not-import-nmp-common, merge-conflict check, uv.lock drift check). Three hooks failed only for local toolchain/environment reasons unrelated to this change and not touching changed files:helm-docs(binary not installed; no Helm changes),uv-lock(local uv 0.9.21 vs required 0.9.14; no dependency changes and theuv.lockdrift check passed), andstudio-lint-staged(local Node engine mismatch / corepack pnpm download; noweb/changes). CI will re-run these on pinned toolchains.Summary by CodeRabbit
New Features
Bug Fixes