feat: validate adapter harness settings - #1
Closed
ZhongxuanWang wants to merge 6 commits into
Closed
Conversation
#### Overview Fix Relay artifact discovery so it follows configured sink filenames and templates instead of reporting every matching file in an output directory. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Honor a valid configured ATOF file-sink `filename`. - Restrict ATIF discovery using its configured `filename_template`, treating non-placeholder characters literally. - Reject absolute or traversing artifact names and resolved artifacts outside their configured directory. - Ignore malformed, missing, and non-file paths. - Preserve the existing glob fallback when an ATOF filename is absent or is not a string. - Canonicalize reported artifact paths so equivalent filesystem paths are represented consistently. - Keep release-branch package versions at `0.1.0`. - Compare generated scaffold dependency paths by filesystem identity so valid Windows extended paths do not fail CI. Fabric-managed configurations already scope output directories by runtime. The primary correction is therefore for user-supplied output directories that are shared or pre-populated, where the previous broad globs could report unrelated JSONL or JSON files. Canonicalization is an observable output change: when an output directory is a symlink, reported artifacts use their resolved paths rather than the configured symlink paths. Legacy Relay observability configuration remains supported and is normalized to the component-based schema. Validation: - Focused Relay artifact tests: **13 passed** - Scaffold end-to-end tests: **3 passed** - Complete Python suite: **484 passed, 43 skipped** - `cargo check -p fabric-python --locked`: **passed** - Python build: **passed** - Full pre-commit suite: **passed** - GitHub Actions: **24/24 checks passed** #### Where should the reviewer start? Start with `collect_relay_artifacts()` and its private path-validation helpers in `adapters/common/src/nemo_fabric_adapters/common/utils.py`. The corresponding cases in `tests/adapters/test_adapaters_common_utils.py` cover configured and missing filenames, literal ATIF templates, non-file entries, resolution failure scenarios, malformed/path-traversal inputs, glob fallback, and path containment. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - None Authors: - Bryan Bednarski (https://github.com/bbednarski9) - David Gardner (https://github.com/dagardner-nv) - gpuCI (https://github.com/GPUtester) - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah) Approvers: - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah) URL: NVIDIA#133
#### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes FABRIC-146 - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit * **Bug Fixes** * Improved release publishing by grouping uploaded wheel files by package. * Prevented duplicate release payloads when multiple wheels belong to the same package. * Ensured each package release includes all of its associated wheel download links. Authors: - David Gardner (https://github.com/dagardner-nv) Approvers: - Yuchen Zhang (https://github.com/yczhang-nv) URL: NVIDIA#135
#### Overview Replace the pre-0.1 portable adapter configuration with one typed, fail-closed `FabricConfig` contract across Claude, Codex, Deep Agents, and Hermes Agent. This change: - replaces the flat system prompt with `instructions.system=InstructionConfig(content=..., mode="replace")`; - moves the optional harness turn limit to `runtime.max_turns`; - replaces the split tool/toolset model with `ToolsConfig(enabled=..., blocked=...)`; - validates every explicitly configured adapter-translated field against the selected adapter descriptor; - keeps `plan(...)`, runtime start, and `run(...)` strict while allowing `doctor(...)` to return structured failed checks for all detected incompatibilities; - clarifies that capability routes assign execution ownership rather than network routing; and - updates the Rust core, Python SDK, CLI presets and scaffolds, bundled adapter descriptors, Harbor integration, examples, schemas, consumer skill, adapter documentation, and generated API references. The v0.1 semantics are deliberate: - `InstructionConfig.mode` supports only `replace`. - `runtime.max_turns=None` preserves the adapter-native default and remains portable. An explicit value requires adapter support; Claude and Hermes map it, while Codex and Deep Agents reject it. - `tools.enabled=None` preserves the harness default; `tools.enabled=[]` disables every executable tool. A name cannot be both enabled and blocked. - Tool names are adapter-native selectors. Consumers use separate `FabricConfig` variants when harnesses name the same logical tool differently. - Claude, Deep Agents, and Hermes enforce normalized tool policy. Codex rejects it because no complete SDK-native mapping exists yet. - No compatibility aliases or transition-only parsing are retained before the 0.1 release. Custom Claude, Codex, and non-native Deep Agents providers must set both `models.<role>.api_key_env` and `models.<role>.base_url`. Native Anthropic and OpenAI authentication remains available. NeMo Fabric does not infer an NVIDIA endpoint or credential in shared adapter code; maintained NVIDIA presets supply those values explicitly. #### Compatibility `Core` means NeMo Fabric owns the behavior and applies it uniformly. `Yes` means the adapter translates and enforces the normalized field. `No` means an explicitly configured value fails planning instead of being ignored. | `FabricConfig` field | Claude | Codex | Deep Agents | Hermes Agent | | --- | --- | --- | --- | --- | | `schema_version` | Core | Core | Core | Core | | `metadata.name`, `.description` | Core | Core | Core | Core | | `harness.adapter_id`, `.resolution` | Core | Core | Core | Core | | `harness.settings` | Adapter-owned escape hatch | Adapter-owned escape hatch | Adapter-owned escape hatch | Adapter-owned escape hatch | | `models.<role>.provider` | Native Anthropic or configured Messages-compatible provider | Native OpenAI or configured Responses-compatible provider | Dynamic LangChain provider | Dynamic Hermes provider | | `models.<role>.model` | Yes | Yes | Yes | Yes | | `models.<role>.api_key_env` | Yes | Yes | Yes | Yes | | `models.<role>.base_url` | Yes | Yes | Yes | Yes | | `models.<role>.temperature` | No | No | Yes | Yes | | `models.<role>.settings.<key>` | No keys declared | No keys declared | No keys declared | No keys declared | | `instructions.system` | Yes | Yes; Codex base instructions | Yes | Yes | | `runtime.input_schema`, `.output_schema` | Core | Core | Core | Core | | `runtime.artifacts`, `.timeout_seconds` | Core | Core | Core | Core | | `runtime.max_turns` | Yes | No | No | Yes; Hermes iteration limit | | `environment.provider`, `.control_location`, `.ownership` | Core | Core | Core | Core | | `environment.workspace`, `.artifacts`, `.env` | Core | Core | Core | Core | | `environment.connection`, `.metadata`, `.settings` | Environment-provider-owned | Environment-provider-owned | Environment-provider-owned | Environment-provider-owned | | `tools.enabled`, `.blocked` | Yes | No | Yes | Yes; Hermes toolset selectors | | `skills.paths` | Yes | Yes | Yes | Yes | | Harness-native MCP servers | Yes | Yes | Yes | Yes | | Fabric-managed MCP servers | No | No | No | No | | `telemetry.providers.relay` | Yes | Yes | Yes | Yes | | `telemetry.providers.native` | No | Yes; OpenTelemetry | Yes; OpenTelemetry and OpenInference | No | | `relay.project`, `.output_dir`, `.observability` | Yes | Yes | Yes | Yes | | `relay.components`, `.policy` | Yes | Yes | Yes | Yes | #### Breaking changes This intentionally replaces unreleased pre-0.1 names rather than preserving aliases: - `system_prompt` becomes `instructions.system`. - top-level `max_turns` becomes `runtime.max_turns`. - `ToolsetConfig` and `tools.toolsets` are removed; `ToolsConfig` owns `enabled` and `blocked` directly. - Harbor inputs use `fabric_system_instruction` and `fabric_enabled_tools`. #### Validation - `just test-rust` — passed: 22 CLI tests, 1 frontier preset test, 36 core tests, and Rust doc tests. - `just test-python` — passed after merging current `release/0.1`: 501 passed, 15 skipped. - `cargo check -p fabric-python --locked` — passed. - `just docs` — generated Python and Rust references, Fern validation, and strict broken-link validation passed. The authenticated redirect check was skipped because `FERN_TOKEN` is not available locally. - Copyright, Ruff, Cargo lock freshness, uv lock freshness, and attribution checks passed for the relevant staged changes. - `cargo fmt --all -- --check` and `git diff --check` — passed. #### Where should the reviewer start? Start with `crates/fabric-core/src/config.rs` for the public shape, descriptor compatibility, tool routing, and strict planning behavior. Then review `crates/fabric-core/src/doctor.rs` for diagnostic planning, `python/src/nemo_fabric/models.py` for the SDK surface, and the Claude and Deep Agents adapters for execution-time tool enforcement. The canonical adapter matrix is in `adapters/README.md`; the consumer contract is in `docs/sdk/python.mdx`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Closes [FABRIC-131](https://linear.app/nvidia/issue/FABRIC-131) - Relates to [FABRIC-64](https://linear.app/nvidia/issue/FABRIC-64) - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit * **New Features** * Added typed instruction handling, runtime limits (timeout/turns), model endpoint selection, harness-visible environment variables (with persisted redaction), and adapter-native tool allow/deny policy. * Added adapter compatibility diagnostics (config- and capability-level) and stricter validation during planning/doctor. * Extended support for custom model providers/endpoints and relay gateway base URL configuration. * **Bug Fixes** * Improved propagation of environment variables into local runtime and harness/tool execution, including correct normalized env handling. * **Documentation** * Updated quickstarts, examples, adapter guidance/compatibility matrices, schemas, and API references to match the expanded configuration model. Authors: - Ajay Thorve (https://github.com/AjayThorve) - David Gardner (https://github.com/dagardner-nv) - gpuCI (https://github.com/GPUtester) Approvers: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah) URL: NVIDIA#117
#### Overview Applies the `::r` NVIDIA documentation style review follow-up to the human-authored documentation changed by NVIDIA#115. The review found five style issues across two files, and this PR corrects all five without changing product behavior. #### Details - Add complete lead-in sentences before API tables. - Use `can` instead of `may` for possibility. - Replace vague wording such as "additive" and "generically" with plain English. - Replace a semicolon with shorter, direct sentences. - Keep generated API reference pages and technical API auditing outside this style-only scope. #### Validation - `uvx --from rust-just just docs` — passed with the existing Fern warning about the generated Python reference SPDX comment; Fern skipped the missing-redirects check. - `git diff --check` — passed. - `::r` changed-line scan — no high-signal style violations remain in the reviewed human-authored documentation. Breaking changes: None. #### Where should the reviewer start? Start with `docs/sdk/python.mdx`, which contains four of the five style corrections. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: NVIDIA#115 - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit * **Documentation** * Clarified the Python SDK “API Inventory” lead-in and tightened wording for `Fabric.doctor(...)`. * Rephrased guidance for custom fields and adapter settings, emphasizing human-readable agent identity and caller-owned annotations. * Clarified that NeMo Fabric preserves these metadata values in the resolved configuration (without copying them into `RunResult.metadata`). * Refined the introductory sentence for the `Fabric` methods reference table. Authors: - Lawrence Lane (https://github.com/lbliii) - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) Approvers: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) URL: NVIDIA#131
#### Overview Follow up on the `::r` documentation style review of NVIDIA#112 by clarifying the NeMo Relay streaming lifecycle guidance in the consumer integration skill. ##### Details - Split a semicolon-linked statement into shorter sentences. - Split a 31-word correlation sentence into three direct sentences. - Preserve the terminology and behavior documented by NVIDIA#112. - Leave generated API references unchanged. ##### Validation - `git diff --check` - `uvx --from rust-just just docs` (passes with the existing Fern warning for the generated Python API index and skipped redirects check) ##### Breaking changes None. #### Where should the reviewer start? Review `skills/integrations/consumer/nemo-fabric-integrate/SKILL.md`, especially the NeMo Relay streaming lifecycle bullet. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to NVIDIA#112 - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit - **Documentation** - Clarified NeMo Relay “Choose A Lifecycle” streaming behavior, including how listener output is limited to the matched scope tree. - Explained record correlation differences between in-process harnesses and gateway harnesses. - Reworded the lifecycle guidance around ATOF record/queue limits and confirmed that earlier-turn records are not carried over. Authors: - Lawrence Lane (https://github.com/lbliii) - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) Approvers: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) URL: NVIDIA#132
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
ZhongxuanWang
force-pushed
the
feat/validate-harness-settings
branch
from
July 28, 2026 20:27
07ff4f3 to
c529942
Compare
Author
|
Superseded by NVIDIA#146 now that NVIDIA#117 has merged into release/0.1. The official draft is the authoritative FABRIC-132 review. |
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.
Overview
Important
This is a stacked draft PR on NVIDIA/NeMo-Fabric#117 and must not merge before it. The base branch belongs to Ajay's fork because PR NVIDIA#117's head branch does not exist in the NVIDIA repository. After NVIDIA#117 merges, this change will be submitted as a replacement PR against
NVIDIA/NeMo-Fabric:release/0.1; GitHub cannot retarget this PR across repositories.Validate
harness.settingsduring planning against JSON Schema owned by the exact resolved adapter descriptor.AdapterDescriptorwith an optional embeddedsettings_schema.setting_sources,max_budget_usd, andpermission_mode.Per FABRIC-132's accepted phased scope, Codex, Deep Agents, and Hermes do not receive production schemas here. Their non-empty
harness.settingsremain fail-closed until follow-up work.This adds
jsonschema0.49.2 to the Rust core with default features disabled. A standards-compliant validator is needed to validate arbitrary descriptor-owned schemas without hard-coded adapter tables, separate schema lookup, or importing adapter Python code. The lockfile adds 87 transitive Rust packages; the explicit license audit classifies all of them under accepted permissive licenses, and there is no Python dependency delta. Most of the apparent line count is generated dependency metadata:ATTRIBUTIONS-Rust.mdandCargo.lock.Details
ADAPTER_PYTHON, or agent-local descriptor atomically with its schema.UnknownAdapterprecedence over settings validation.Validation
cargo fmt --all -- --checkcargo check -p fabric-python --lockedjust --set no_uv true build-pythonjust test-rust: 22 CLI, 1 frontier, and 42 core tests passedjust test-python: 515 passed, 15 skippedjust docswith Rust 1.93: all checks passed; only the expected unauthenticated Fern redirect-check warningpre-commit run --all-files: all hooks passedscripts/licensing/license_diff.py --base-ref upstream/pr/117-live: 87 permissively licensed Rust additions; no Python additions, removals, or changesgit diff --checkassertionsWhere should the reviewer start?
Start with
validate_harness_settingsand descriptor loading incrates/fabric-core/src/config.rs, then reviewadapters/claude/fabric-adapter.jsonandtests/python/test_harness_settings_validation.py. The installed and override discovery coverage is intests/python/test_installed_adapter_discovery.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to FABRIC-132
Depends on NVIDIA/NeMo-Fabric#117
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.