fix: tighten OpenClaw ATOF hook-backed provenance metadata#209
Conversation
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
WalkthroughThis PR adds structured metadata to OpenClaw hook-replay events, enriches session state with stable identifiers used in emitted spans/marks, updates emission points to forward metadata, extends test mocks to capture metadata, and removes several Node attributions while consolidating Rust license entries. ChangesLicense Attribution Cleanup
OpenClaw Hook-Replay Metadata Enrichment
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
github-actions
Bot
added
the
lang:js
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ATTRIBUTIONS-Rust.md`:
- Around line 3126-3127: The generated ATTRIBUTIONS-Rust.md is missing blank
lines before the "### License:" headings (MD022); update the generator in
scripts/licensing/attributions_lockfile_md.py to always emit a separating blank
line before the license heading. Locate the markdown-emission function (e.g.,
generate_markdown / emit_entry / format_license_section or the function that
builds the package block / write_entry) and ensure it inserts a "\n" (or an
empty line) before emitting "### License: ..." for each package entry,
preserving the existing license-selection logic that prefers Apache-2.0 where
you consolidate dual-license crates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e8ea1d7c-9590-41ee-9677-0710a6555bce
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
ATTRIBUTIONS-Node.mdATTRIBUTIONS-Rust.mdintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
💤 Files with no reviewable changes (1)
- ATTRIBUTIONS-Node.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{wasm,js,ts}{,x}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure WebAssembly package naming conventions are consistent with generated package expectations and downstream consumption
Files:
integrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run Node.js formatting with
npm run format --workspace=nemo-relay-nodeInclude SPDX license header in all JavaScript and TypeScript source files using double-slash comment syntax
Files:
integrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
integrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.tsATTRIBUTIONS-Rust.md
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
integrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
**/*{test,spec,smoke}.{js,ts,py}
📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)
Relevant integration tests or smoke path must pass
Files:
integrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
integrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
ATTRIBUTIONS-Rust.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
ATTRIBUTIONS-Rust.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)
Documentation must be updated if activation or usage changed
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links ...
Files:
ATTRIBUTIONS-Rust.md
**/{docs,examples,**/*.md,*.patch,*.diff,.github,*.sh,*.yaml,*.yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update documentation, examples, CI configuration, and patch artifacts when performing rename operations
Files:
ATTRIBUTIONS-Rust.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
ATTRIBUTIONS-Rust.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
ATTRIBUTIONS-Rust.md
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
ATTRIBUTIONS-Rust.md
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
ATTRIBUTIONS-Rust.md
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
ATTRIBUTIONS-Rust.md
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
ATTRIBUTIONS-Rust.md
🧠 Learnings (16)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
integrations/openclaw/src/hook-replay/tool.tsintegrations/openclaw/src/hook-replay/marks.tsintegrations/openclaw/src/hooks-backend.tsintegrations/openclaw/src/hook-replay/llm.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
📚 Learning: 2026-05-21T22:52:14.330Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/validate-change/SKILL.md:0-0
Timestamp: 2026-05-21T22:52:14.330Z
Learning: For Node.js binding changes, use `test-node-binding`
Applied to files:
integrations/openclaw/test/hooks-backend.test.ts
📚 Learning: 2026-05-29T21:25:49.977Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-29T21:25:49.977Z
Learning: Applies to crates/node/**/*.{js,ts,tsx} : Node.js public entry points include the main runtime package plus `nemo-relay-node/typed`, `nemo-relay-node/plugin`, and `nemo-relay-node/adaptive`.
Applied to files:
integrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.ts
📚 Learning: 2026-05-21T22:47:58.898Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/add-middleware/SKILL.md:0-0
Timestamp: 2026-05-21T22:47:58.898Z
Learning: Applies to crates/core/src/api/runtime/state.rs : Add chain execution helpers to `NemoRelayContextState` following the pattern of existing methods like `tool_sanitize_request_chain` or `tool_request_intercepts_chain`
Applied to files:
integrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.ts
📚 Learning: 2026-05-29T21:25:49.977Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-29T21:25:49.977Z
Learning: Scope stacks are hierarchical and always have a root scope. They establish parent-child event relationships, visibility for scope-local middleware and subscribers, cleanup boundaries, and concurrent request isolation.
Applied to files:
integrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.ts
📚 Learning: 2026-05-26T21:03:12.012Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/review-doc-style/SKILL.md:0-0
Timestamp: 2026-05-26T21:03:12.012Z
Learning: Keep documentation aligned with current NeMo Relay behavior, repo layout, and entry points
Applied to files:
integrations/openclaw/test/llm-replay.test.ts
📚 Learning: 2026-05-29T21:25:49.977Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-29T21:25:49.977Z
Learning: Events use ATOF `0.1` as the canonical event format. Scope events use start/end pairs; mark events record runtime checkpoints.
Applied to files:
integrations/openclaw/test/tool-replay.test.ts
📚 Learning: 2026-05-21T22:51:50.794Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/update-project-version/SKILL.md:0-0
Timestamp: 2026-05-21T22:51:50.794Z
Learning: Regenerate `ATTRIBUTIONS-Rust.md` with `./scripts/generate_attributions.sh rust` after Cargo metadata changes
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-21T22:51:50.794Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/update-project-version/SKILL.md:0-0
Timestamp: 2026-05-21T22:51:50.794Z
Learning: Do not forget to update `Cargo.lock`, `ATTRIBUTIONS-Rust.md`, and `ATTRIBUTIONS-Node.md` after changing versioned inputs that feed them
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-27T02:34:30.939Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-05-27T02:34:30.939Z
Learning: Applies to **/README.md : Update relevant crate or package README when that surface changed
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-26T21:03:12.012Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/review-doc-style/SKILL.md:0-0
Timestamp: 2026-05-26T21:03:12.012Z
Learning: Applies to **/README.md|docs/index.md|python/nemo_relay/README.md|crates/*/README.md : Public behavior changes must be reflected in corresponding entry-point documentation (Must-Fix)
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-27T02:34:30.939Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-05-27T02:34:30.939Z
Learning: Applies to **/*.rs : Include SPDX license header in all Rust source files using double-slash comment syntax
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-21T22:52:14.330Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/validate-change/SKILL.md:0-0
Timestamp: 2026-05-21T22:52:14.330Z
Learning: Applies to {docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md} : Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-21T22:49:46.956Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/rename-surfaces/SKILL.md:0-0
Timestamp: 2026-05-21T22:49:46.956Z
Learning: Applies to **/*.{rs,toml} : Update Rust crate names and module prefixes during coordinated rename operations
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-21T22:50:51.194Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/test-ffi-surface/SKILL.md:0-0
Timestamp: 2026-05-21T22:50:51.194Z
Learning: Applies to **/*.rs : Run `just test-rust` to validate FFI changes
Applied to files:
ATTRIBUTIONS-Rust.md
📚 Learning: 2026-05-21T22:47:33.109Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/add-binding-feature/SKILL.md:0-0
Timestamp: 2026-05-21T22:47:33.109Z
Learning: Applies to crates/wasm/src/api/**/*.rs : Update WebAssembly binding in `crates/wasm/src/api/mod.rs` for language-native bindings
Applied to files:
ATTRIBUTIONS-Rust.md
🪛 markdownlint-cli2 (0.22.1)
ATTRIBUTIONS-Rust.md
[warning] 3127-3127: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 3127-3127: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 7902-7902: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 7902-7902: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 8559-8559: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 8559-8559: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20346-20346: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 20346-20346: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 38802-38802: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 38802-38802: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (18)
integrations/openclaw/src/hook-replay/llm.ts (2)
555-557: LGTM!
796-804: LGTM!integrations/openclaw/src/hook-replay/tool.ts (2)
70-78: LGTM!
92-94: LGTM!integrations/openclaw/src/hook-replay/session.ts (3)
253-264: LGTM!
333-353: LGTM!
275-293: ⚡ Quick winRe-check positional-argument safety for
closeSessionRoot
- Within this repo, the only
closeSessionRoot(...)call is inintegrations/openclaw/src/hooks-backend.ts(~line 388), where the 5th positional argument passed ismetadata(andtimestampis omitted), so the positional “old timestamp → new metadata” corruption can’t occur internally.closeSessionRootis still exported fromintegrations/openclaw/src/hook-replay/session.ts, so downstream (non-TS-checked) callers could still pass the old 5th positional argument; a consumer-side search/validation is needed.integrations/openclaw/src/hook-replay/marks.ts (1)
17-33: LGTM!integrations/openclaw/src/hooks-backend.ts (3)
122-132: LGTM!
386-391: LGTM!
402-409: LGTM!integrations/openclaw/test/hooks-backend.test.ts (3)
34-53: LGTM!Also applies to: 82-93, 164-169, 201-209, 265-272, 298-305, 333-341, 344-377
517-526: LGTM!
500-506: ⚡ Quick winCheck
pushScope/eventargument ordering formetadatain the real runtime APIThe current stubs assume
metadatais the last positional parameter forpushScope((name, scopeType, _handle, _attributes, data, _links, metadata)) and the 4th argument forevent((name, handle, data, metadata)). The available repo/doc evidence didn’t surface the exactNemoRelayRuntimeModule.pushScope/eventsignatures or the OpenClaw hook/backend call sites needed to confirm these indices, so the assertions could still pass while capturing the wrong argument.Update the test to derive/cross-check the parameter order directly from the actual exported types (and the concrete backend call signature), or adjust the stub to capture
metadatain a way that can’t silently drift if the call-site order changes.integrations/openclaw/test/llm-replay.test.ts (2)
1103-1115: ⚡ Quick winConfirm
llmCall/llmCallEnd/toolCall/toolCallEndmetadata positions match the real API.Same positional-capture concern as the hooks-backend stubs:
metadatais read as argument 6 ofllmCall, argument 4 ofllmCallEnd, and argument 6 oftoolCall. If the realNemoRelayRuntimeModulesignatures differ, the capturedmetadatais silently the wrong arg. Covered by the verification script on the hooks-backend file.
55-75: LGTM!Also applies to: 530-552, 574-584, 1019-1048, 1144-1152
integrations/openclaw/test/tool-replay.test.ts (1)
46-77: LGTM!Also applies to: 161-181, 199-206, 261-289
ATTRIBUTIONS-Rust.md (1)
13277-13297: LGTM!
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
|
Pushed a follow-up for the runtime-signature feedback.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
integrations/openclaw/test/llm-replay.test.ts (1)
1156-1164: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExtract duplicated helper to shared test utility.
This helper is duplicated across three test files. While the
correlationomission is correct for LLM replay (LLM metadata legitimately includescorrelationper lines 538, 551, 582), extracting to a shared utility with a parameter for optional key exclusions would eliminate duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/openclaw/test/llm-replay.test.ts` around lines 1156 - 1164, Extract the duplicated assertNoOverclaimedReplayMetadata helper into a shared test utility function and add an optional parameter to exclude keys (e.g., exclusions?: string[]). Keep the same default behavior (assert metadata is an object and that keys 'agent_kind','provider_payload_exact','fidelity_source','gateway_path','gateway_route' are absent) but allow callers to pass exclusions like ['correlation'] when LLM replay legitimately includes correlation; update all three tests to import the shared utility and call it with the appropriate exclusions where needed, referencing the original function name assertNoOverclaimedReplayMetadata for locating and replacing usages.integrations/openclaw/test/hooks-backend.test.ts (1)
533-542: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExtract duplicated helper to shared test utility.
assertNoOverclaimedHookMetadatais duplicated across three test files (hooks-backend.test.ts,llm-replay.test.ts,tool-replay.test.ts) with slight variations. Extract to a shared module (e.g.,test/test-utils.ts) to eliminate duplication and ensure consistent validation logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/openclaw/test/hooks-backend.test.ts` around lines 533 - 542, Extract the duplicated helper assertNoOverclaimedHookMetadata into a shared test utility module and update callers to import it: create a new exported function (e.g., assertNoOverclaimedHookMetadata) in a common test-utils file and move the exact validation logic there, then replace the local definitions in hooks-backend.test.ts, llm-replay.test.ts, and tool-replay.test.ts with imports of that shared function; ensure the signature and behavior (checking metadata is object and asserting absence of keys 'agent_kind', 'provider_payload_exact', 'fidelity_source', 'gateway_path', 'gateway_route', 'correlation') remain identical so existing tests keep working.integrations/openclaw/test/tool-replay.test.ts (1)
292-301: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExtract duplicated helper to shared test utility.
This helper is duplicated across three test files. The
correlationcheck here is correct for tool replay (tools should not claim correlation fields), but extracting to a shared utility would eliminate duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/openclaw/test/tool-replay.test.ts` around lines 292 - 301, The function assertNoOverclaimedHookMetadata should be extracted from this test file into a shared test utility module and reused by the three tests that currently duplicate it; create a new exported helper (e.g., in a shared test/utils file) that preserves the same checks including the 'correlation' absence for tool replay, replace the local definition in integrations/openclaw/test/tool-replay.test.ts with an import of that helper, and update the other two test files to import and use the same exported assertNoOverclaimedHookMetadata to eliminate duplication while keeping behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/openclaw/test/hooks-backend.test.ts`:
- Line 454: The pushScope array type is missing a timestamp field, causing
inconsistency with other tests (e.g., llm-replay.test.ts); update the pushScope
type declaration to include timestamp: number (or string as used elsewhere),
then in the test where you destructure the pushed scope (the code that currently
pulls name, scopeType, data, metadata, input) also destructure timestamp and
store it in the pushScope array entries so timestamp is captured alongside the
other fields (adjust the destructuring and the object stored in the pushScope
push/assignment to include timestamp).
---
Outside diff comments:
In `@integrations/openclaw/test/hooks-backend.test.ts`:
- Around line 533-542: Extract the duplicated helper
assertNoOverclaimedHookMetadata into a shared test utility module and update
callers to import it: create a new exported function (e.g.,
assertNoOverclaimedHookMetadata) in a common test-utils file and move the exact
validation logic there, then replace the local definitions in
hooks-backend.test.ts, llm-replay.test.ts, and tool-replay.test.ts with imports
of that shared function; ensure the signature and behavior (checking metadata is
object and asserting absence of keys 'agent_kind', 'provider_payload_exact',
'fidelity_source', 'gateway_path', 'gateway_route', 'correlation') remain
identical so existing tests keep working.
In `@integrations/openclaw/test/llm-replay.test.ts`:
- Around line 1156-1164: Extract the duplicated
assertNoOverclaimedReplayMetadata helper into a shared test utility function and
add an optional parameter to exclude keys (e.g., exclusions?: string[]). Keep
the same default behavior (assert metadata is an object and that keys
'agent_kind','provider_payload_exact','fidelity_source','gateway_path','gateway_route'
are absent) but allow callers to pass exclusions like ['correlation'] when LLM
replay legitimately includes correlation; update all three tests to import the
shared utility and call it with the appropriate exclusions where needed,
referencing the original function name assertNoOverclaimedReplayMetadata for
locating and replacing usages.
In `@integrations/openclaw/test/tool-replay.test.ts`:
- Around line 292-301: The function assertNoOverclaimedHookMetadata should be
extracted from this test file into a shared test utility module and reused by
the three tests that currently duplicate it; create a new exported helper (e.g.,
in a shared test/utils file) that preserves the same checks including the
'correlation' absence for tool replay, replace the local definition in
integrations/openclaw/test/tool-replay.test.ts with an import of that helper,
and update the other two test files to import and use the same exported
assertNoOverclaimedHookMetadata to eliminate duplication while keeping behavior
identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ef64db6c-81e6-45dc-b03a-63ab6252b065
📒 Files selected for processing (4)
integrations/openclaw/src/hook-replay/session.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/test/tool-replay.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*{test,spec,smoke}.{js,ts,py}
📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)
Relevant integration tests or smoke path must pass
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.ts
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.ts
**/*.{wasm,js,ts}{,x}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure WebAssembly package naming conventions are consistent with generated package expectations and downstream consumption
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/src/hook-replay/session.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run Node.js formatting with
npm run format --workspace=nemo-relay-nodeInclude SPDX license header in all JavaScript and TypeScript source files using double-slash comment syntax
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/src/hook-replay/session.ts
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/src/hook-replay/session.ts
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/src/hook-replay/session.ts
🧠 Learnings (1)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
integrations/openclaw/test/tool-replay.test.tsintegrations/openclaw/test/hooks-backend.test.tsintegrations/openclaw/test/llm-replay.test.tsintegrations/openclaw/src/hook-replay/session.ts
🔇 Additional comments (8)
integrations/openclaw/src/hook-replay/session.ts (3)
253-264: LGTM!
275-282: ⚡ Quick wincloseSessionRoot and pushScope argument order are consistent
closeSessionRootpositional parameters: the only in-repo call site (integrations/openclaw/src/hooks-backend.ts) passesmetadataas the 5th argument and does not pass a positionaltimestamp(6th arg), so no old 5-argtimestampusage remains.pushScopeargument order inintegrations/openclaw/src/hook-replay/session.tsis consistent withParameters<NemoRelayRuntimeModule['pushScope']>(runtime/test tuple expects…, data, metadata, input, timestamp)), so thedata/metadatapositions are not swapped.
343-352: ⚡ Quick winConfirm
pushScopepositional arg semantics (data, metadata, input, timestamp).OpenClaw’s tests stub
pushScope(...args)as[name, scopeType, , , data, metadata, input, timestamp], and thesession.tscall passes arg5=data, arg6=metadata, arg7=data(asinput), arg8=input.timestamp ?? null. Output is provided viapopScope(...), not as apushScopepositional argument.> Likely an incorrect or invalid review comment.integrations/openclaw/test/hooks-backend.test.ts (2)
41-47: LGTM!Also applies to: 94-98
512-522: LGTM!integrations/openclaw/test/llm-replay.test.ts (1)
1097-1100: LGTM!Also applies to: 1104-1127
integrations/openclaw/test/tool-replay.test.ts (2)
46-54: LGTM!Also applies to: 68-77, 172-181
255-289: LGTM!
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
dagardner-nv
left a comment
dagardner-nv
left a comment
There was a problem hiding this comment.
Approved for deps, since the changes to package-lock.json don't introduce any actual dep changes
|
/merge |
3 participants
Overview
This PR tightens provenance metadata for OpenClaw hook-backed ATOF events and adds focused test coverage so hook-backed events emit the stable metadata they actually have, without claiming provider-fidelity fields that the current hook-backed path does not expose.
Details
agent_kind,provider_payload_exact, orfidelity_source.package-lock.json,ATTRIBUTIONS-Node.md, andATTRIBUTIONS-Rust.mdso the full pre-commit suite remains green.Where should the reviewer start?
Start with
integrations/openclaw/src/hooks-backend.ts, then review the metadata plumbing inintegrations/openclaw/src/hook-replay/session.ts,integrations/openclaw/src/hook-replay/llm.ts, andintegrations/openclaw/src/hook-replay/tool.ts, followed by the targeted OpenClaw tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Validation
npm test --workspace=nemo-relay-openclawnpm run typecheck --workspace=nemo-relay-openclawcargo test -p nemo-relay atofcargo test -p nemo-relayuv run pre-commit run --all-filesSummary by CodeRabbit
Improvements
Chores