fix: preserve Codex automation tool outputs - #1016
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (44)
🧰 Additional context used📓 Path-based instructions (29)Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.⚙️ CodeRabbit configuration file Files:
Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.⚙️ CodeRabbit configuration file Files:
For changes affecting `crates/core`, `crates/adaptive`, or shared Rust runtime semantics, expand validation to the full binding matrix with `validate-change`.📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md) Files:
If a language surface changed, always run that language's test target even when Rust core did not change.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Keep async behavior on the existing tokio-based model.📄 CodeRabbit inference engine (AGENTS.md) Files:
[ ] Do all bindings expose the same logical knobs and semantics?📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md) Files:
If any Rust code changed, always run `just test-rust`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
**Formatting**: `cargo fmt` (rustfmt defaults) **Linting**: `cargo clippy -- -D warnings` -- all warnings are treated as errors📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
If any Rust code changed, also run `cargo fmt --all`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Use `Json = serde_json::Value` in Rust-facing runtime APIs where the existing code expects JSON payloads.📄 CodeRabbit inference engine (AGENTS.md) Files:
[ ] Branch scope is coherent and reviewable [ ] Relevant tests passed under `validate-change` [ ] Docs and examples updated for any public behavior changes [ ] Pull request title follows Conventional Commit style and uses the correct type U...📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md) Files:
Format changed files with the language-native formatter before the final lint/test pass.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Keep NeMo Relay optional Use stable, documented framework or plugin APIs Wrap tool and LLM paths at the correct framework boundary Preserve the framework's original behavior when NeMo Relay is absent Integration uses public framework or plu...📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md) Files:
Tool execution callbacks and each execution-intercept `next` continuation return the canonical `ToolExecutionResult { result, annotation }`.📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md) Files:
For changes in the Rust core, adaptive, dynamic plugin, worker, worker-proto, or types crates, run `cargo fmt --all`, `just test-rust`, and `cargo clippy --workspace --all-targets -- -D warnings` as the default validation sequence.📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md) Files:
[ ] `crates/core` or `crates/adaptive` changes ran the full language matrix📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md) Files:
For shared-semantics or broad runtime changes in the core or adaptive crates, run `just ci=true test-rust`.📄 CodeRabbit inference engine (.agents/skills/test-rust-core/SKILL.md) Files:
**Core Rust** Implement the behavior first in `crates/core/src/api/` and related core modules such as `crates/core/src/api/runtime/`, `crates/core/src/codec/`, or `crates/core/src/json.rs`.📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md) Files:
Keep SPDX headers on source, docs, scripts, and configuration files.📄 CodeRabbit inference engine (AGENTS.md) Files:
**Validation** Run the validation matrix from the `validate-change` skill for the affected surfaces.📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md) Files:
Use `test-ffi-surface`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
[ ] Any Rust change ran `just test-rust` [ ] Any Rust change ran `cargo fmt --all` [ ] Any Rust change ran `cargo clippy --workspace --all-targets -- -D warnings`📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md) Files:
Follow binding naming conventions: Rust and Python `snake_case`, C FFI exports prefixed `nemo_relay_`, Go `PascalCase` for public APIs, Node.js `camelCase`.📄 CodeRabbit inference engine (AGENTS.md) Files:
[ ] SPDX license header on any new files📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md) Files:
Update docs and examples in the same branch.📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md) Files:
If the change touched `crates/core` or shared runtime semantics, also use `validate-change` for broader validation📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md) Files:
Run `cargo fmt --all` for all FFI work since it is Rust work Run `just test-rust` to validate FFI changes Run `cargo clippy --workspace --all-targets -- -D warnings` to enforce strict linting on FFI work📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md) Files:
Run `cargo fmt --all` when Rust files are changed as part of Node work Run `cargo clippy --workspace --all-targets -- -D warnings` when Rust files are changed as part of Node work Run `just test-rust` when Rust files are changed as part of...📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md) Files:
When Rust files changed as part of Go work, also run `cargo fmt --all`, `just test-rust`, and `cargo clippy --workspace --all-targets -- -D warnings`📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md) Files:
🔇 Additional comments (2)
WalkthroughThe decoder detects Codex metadata, passes the dialect context to input-item decoding, and preserves valid Codex ChangesCodex input decoding
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change preserves valid Codex tool outputs that omit public call IDs while retaining strict validation for standard and malformed requests. The covered decoding and round-trip behavior presents no remaining merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ling-with-nemo-relay
|
/merge |
Overview
Allow Codex scheduled and automation tasks to pass through Relay when Codex persists a namespaced app-tool result without the public Responses API call ID. The compatibility path is restricted to requests carrying Codex installation metadata and preserves the native item losslessly.
Details
client_metadata.x-codex-installation-id.function_call_outputitems as provider-native only whencall_idis absent or null and validname,namespace, andoutputfields are present.Validation:
cargo fmt --all -- --checkcargo test -p nemo-relay openai_responses --lib(57 passed)cargo test -p nemo-relay --lib(1,599 passed)cargo clippyandcargo check, which are blocked by existing unresolvedinitialize_pluginsandclear_plugin_configurationreferences innemo-relay-pii-redactiontests.just test-rustis blocked by the same unrelatednemo-relay-pii-redactioncompilation errors.Breaking changes: none.
Where should the reviewer start?
Start with
decode_responses_input_itemincrates/core/src/codec/openai_responses.rs, then reviewcodex_namespaced_function_output_without_call_id_round_trips_losslesslyin the corresponding codec tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit