mirror of
https://github.com/NVIDIA/NemoClaw.git
synced 2026-07-03 03:37:16 +00:00
<!-- markdownlint-disable MD041 --> ## Summary Retires the remaining legacy shell-driven E2E lanes for #5098 Phase 11 and makes the surviving E2E surface a single target/live Vitest workflow. This terminal cleanup deletes the old runner paths, consolidates fixtures and workflow controls, and preserves the operational, docs-validation, and two-agent security-posture coverage landed while the migration was in flight. ## Related Issues - Parent migration: #5098 - Cutover acceptance and post-merge burn-in: #5919 ## Changes - Replace `.github/workflows/e2e-vitest-scenarios.yaml` with `.github/workflows/e2e.yaml`; remove the legacy nightly/script workflows and shared script action. - Move E2E fixtures, live tests, registry, manifests, support tests, and migration docs under `test/e2e/` with target/live naming. - Delete converted `test/e2e/test-*.sh` entrypoints and the retired shell-runner test/support code while retaining implementation shell fixtures used by Vitest. - Rename target advisor files/schema under `tools/e2e-advisor/`, remove auto-dispatch, and align PR Review Advisor terminology. - Preserve #6012 controls: inventory-derived selection, fail-closed selectors, complete aggregation, PR reporting, scheduled failure issues, timing sanitization, and scorecards. - Preserve #6013 behavior: multiline sandbox scripts plus default-enabled `docs-validation` and two-agent `security-posture` jobs in the final workflow/path/project model. - Keep parity decisions and execution evidence in #5919; exact-head full, explicit-only, malformed-selector, and selective-dispatch runs are linked there. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: exact-head CodeQL/GHAS and #5919 domain approvals are pending - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: CodeRabbit could not perform a line review because the 409-file cutover exceeds its 300-file service limit; exact-head GPT-5.5 and Nemotron findings are dispositioned in PR comments, while human #5919 domain review remains pending ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [x] Full `npm test` passes (broad runtime changes only) - [ ] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with 0 errors and 2 existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
225 lines
11 KiB
YAML
225 lines
11 KiB
YAML
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
|
|
# SPDX-License-Identifier: Apache-2.0
|
|
|
|
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
|
|
|
|
# Keep durable project guidance in the nearest AGENTS.md. CodeRabbit discovers
|
|
# those files automatically, so path instructions here are reserved for review
|
|
# gaps that need a narrower, change-specific lens.
|
|
language: "en-US"
|
|
early_access: false
|
|
reviews:
|
|
profile: "chill"
|
|
request_changes_workflow: false
|
|
high_level_summary: true
|
|
in_progress_fortune: false
|
|
poem: false
|
|
review_status: true
|
|
review_details: false
|
|
auto_review:
|
|
enabled: true
|
|
drafts: false
|
|
|
|
# E2E recommendations intentionally do not live here. The E2E Advisor derives
|
|
# them from each PR diff and the current workflows instead of a duplicated,
|
|
# manually synchronized path-to-job catalog.
|
|
path_instructions:
|
|
- path: "src/**"
|
|
instructions: |
|
|
Apply a migration-completion review whenever a PR introduces a
|
|
replacement path, architecture, state model, or framework.
|
|
|
|
- Trace every in-scope entrypoint and lifecycle path, including fresh
|
|
execution, resume/retry/rebuild, persisted state, scripts, tests, docs,
|
|
and workflow wiring. The new path existing is not evidence of cutover.
|
|
- Require in-scope callers to use one authoritative path and delete the
|
|
superseded runtime path, forwarding glue, support helpers, and tests in
|
|
the same PR unless it is in an explicitly bounded compatibility or
|
|
confidence window.
|
|
- Retain an old path only for a demonstrated external/persisted-data
|
|
contract or a bounded confidence/rollback window. Keep the replacement
|
|
authoritative, freeze the old path against new callers and features, link
|
|
the retirement issue or PR in GitHub, and state observable exit criteria.
|
|
- If a PR intentionally migrates only a slice, it must say so and link the
|
|
remaining work in GitHub. Do not introduce repository-local migration
|
|
ledgers or describe the overall migration as complete.
|
|
- Tests must prove that public entrypoints reach the new path and that the
|
|
old path is deleted or cannot execute.
|
|
|
|
- path: "src/lib/{actions,domain,adapters,state}/**"
|
|
instructions: |
|
|
Review ownership against `src/lib/README.md`: actions orchestrate, domain
|
|
modules make pure decisions, adapters own host/process/network boundaries,
|
|
and state modules own persisted files and state I/O. Flag cross-layer
|
|
cycles, duplicate sources of truth, and forwarding wrappers that add a new
|
|
layer without retiring the old owner and its callers.
|
|
|
|
- path: "src/{commands,lib/cli}/**"
|
|
instructions: |
|
|
Review this change against the single-path oclif architecture.
|
|
|
|
- Command classes own grammar, parsing, help, and translation into typed
|
|
action inputs. Behavior and orchestration belong in `src/lib/actions/**`.
|
|
- Flag manual argv parsing, ad hoc command routing, rebuilding string argv
|
|
after oclif has parsed it, or direct platform/registry/credential work in
|
|
a command class.
|
|
- Keep `src/lib/cli/**` limited to framework, metadata, routing, and help
|
|
infrastructure rather than product behavior.
|
|
|
|
- path: "src/nemoclaw.ts"
|
|
instructions: |
|
|
This file is a compatibility front controller, not a command router.
|
|
Keep it limited to loading and exposing `dispatchCli`. Flag new command
|
|
grammar, branching, lifecycle behavior, or manual parsing here. If the
|
|
final caller of a compatibility export is removed, require the export and
|
|
its tests to be deleted in the same PR.
|
|
|
|
- path: "src/lib/{onboard.ts,onboard/**,state/onboard-*.ts}"
|
|
instructions: |
|
|
Review onboarding and resume behavior against the target architecture in
|
|
`src/lib/onboard/machine/README.md`.
|
|
|
|
- Keep `src/lib/onboard.ts` as entry setup and dependency wiring. State
|
|
sequencing, prompts, repair decisions, and phase effects belong in state
|
|
handlers or focused services.
|
|
- `OnboardRuntime` owns machine transitions. Step helpers record step
|
|
status; flag any expansion of direct machine mutation escape hatches.
|
|
- Resume and repair bridges must correspond to real persisted older-session
|
|
shapes, be idempotent across interruption/replay, keep secrets redacted,
|
|
and converge on the same authoritative path as a fresh run.
|
|
- A migrated phase must remove its old sequencing branch and bridge helpers,
|
|
with fresh, resumed, repair, and failure coverage at the public boundary.
|
|
|
|
- path: "src/lib/messaging/**"
|
|
instructions: |
|
|
Review against the manifest-first architecture in
|
|
`src/lib/messaging/AGENTS.md`.
|
|
|
|
- Channel behavior belongs in manifests, resolvers, hooks, and appliers;
|
|
onboard and sandbox actions should only plan and orchestrate.
|
|
- A channel migration must remove its duplicated provider, policy, render,
|
|
credential, and runtime logic from legacy onboarding, rebuild, scripts,
|
|
and generated-config paths. Transitional tables must be derived from the
|
|
manifest registry rather than maintained independently.
|
|
- Verify persisted-plan hydration and parity across onboard, add/remove,
|
|
start/stop, rebuild, resume, diagnostics, and build-time application.
|
|
- Plans and persisted state must remain serializable and secret-free.
|
|
|
|
- path: "src/lib/{sandbox/**,actions/sandbox/**,state/sandbox.ts}"
|
|
instructions: |
|
|
Review sandbox behavior against the layer ownership in `src/lib/README.md`.
|
|
|
|
- `src/lib/sandbox/**` is transitional support code, not a new home for
|
|
workflow orchestration. Actions own lifecycle workflows, domain modules
|
|
own pure decisions, adapters own Docker/OpenShell/process calls, and state
|
|
modules own persisted registry data.
|
|
- When moving a sandbox operation to an action, require every command and
|
|
internal caller to use it and delete the superseded helper path rather
|
|
than leaving two lifecycle implementations.
|
|
- Destructive lifecycle operations must validate before mutation, preserve
|
|
state/backup invariants, and cover failure, recovery, rebuild, and resume
|
|
behavior without bypassing the public action boundary.
|
|
|
|
- path: "src/lib/{security,credentials,shields}/**"
|
|
instructions: &security-boundary |
|
|
Treat this as a security boundary.
|
|
|
|
- Trace untrusted input, credential material, filesystem paths, subprocess
|
|
arguments, and network targets across the full changed flow.
|
|
- Preserve deny-by-default behavior, least privilege, redaction, and
|
|
fail-closed handling. Do not weaken a guard only to retain legacy behavior.
|
|
- Prefer argv arrays and structured APIs over shell command construction.
|
|
- Require negative-path tests that prove the boundary rejects bypasses and
|
|
does not leak secrets in errors, logs, state, or process arguments.
|
|
|
|
- path: "src/lib/sandbox/{config,privileged-exec}.ts"
|
|
instructions: *security-boundary
|
|
|
|
- path: "nemoclaw/src/security/**"
|
|
instructions: *security-boundary
|
|
|
|
- path: "nemoclaw/src/blueprint/ssrf.ts"
|
|
instructions: *security-boundary
|
|
|
|
- path: "Dockerfile*"
|
|
instructions: *security-boundary
|
|
|
|
- path: "agents/**"
|
|
instructions: *security-boundary
|
|
|
|
- path: "scripts/nemoclaw-start.sh"
|
|
instructions: *security-boundary
|
|
|
|
- path: "scripts/lib/sandbox-init.sh"
|
|
instructions: *security-boundary
|
|
|
|
- path: "nemoclaw-blueprint/scripts/http-proxy-fix.js"
|
|
instructions: *security-boundary
|
|
|
|
- path: "nemoclaw-blueprint/policies/**"
|
|
instructions: *security-boundary
|
|
|
|
- path: "test/e2e/**"
|
|
instructions: &e2e-migration |
|
|
Review against the E2E guide in `test/e2e/`. Vitest is the one E2E
|
|
execution path, and fixtures are support code rather than another runner.
|
|
|
|
- Preserve real shell, process, installer, platform, and full-journey
|
|
boundaries by invoking them from Vitest when they are the contract.
|
|
- Flag any new top-level `test/e2e/test-*.sh` entry point, parallel E2E
|
|
workflow, or wrapper that recreates a second execution lane.
|
|
- Keep migration status and ownership in GitHub issues and PRs. Do not add a
|
|
repository-local inventory, checklist, or parallel status model.
|
|
- Flag new runners, compilers, fixture frameworks, or generalized registries
|
|
when a focused Vitest test and local helper would express the behavior.
|
|
|
|
- path: ".github/workflows/e2e.yaml"
|
|
instructions: *e2e-migration
|
|
|
|
- path: "**/*.test.{ts,js,mts,mjs,cts,cjs}"
|
|
instructions: |
|
|
Review tests for behavioral confidence rather than implementation lock-in.
|
|
|
|
- Prefer observable outcomes through the public boundary over source-text,
|
|
private-shape, or mock-call assertions.
|
|
- Flag copied production algorithms, broad mocks that bypass the behavior
|
|
under test, and conditionals that make a test pass without exercising its
|
|
claim.
|
|
- Migration tests must prove the superseded path is unreachable or removed,
|
|
not merely prove that the new path also works.
|
|
|
|
- path: ".github/workflows/**"
|
|
instructions: |
|
|
Review workflow changes as trusted automation.
|
|
|
|
- A `pull_request_target` workflow must not check out, import, install, or
|
|
execute PR-controlled code while holding base-repository secrets or write
|
|
permissions.
|
|
- Keep permissions least-privileged and pass untrusted values as data rather
|
|
than interpolating them into shell programs.
|
|
- Derive job inventories and aggregate dependencies from one source of truth
|
|
or validate them deterministically. Do not add another manually maintained
|
|
path-to-job mirror in `.coderabbit.yaml`.
|
|
|
|
- path: "scripts/checks/**"
|
|
instructions: &guardrail |
|
|
Review guardrails and advisors as product code, not policy prose.
|
|
|
|
- Enforce objective invariants with deterministic code. Reserve model prompts
|
|
for judgment that cannot be computed reliably.
|
|
- Derive inventories and limits from a canonical source where possible; flag
|
|
duplicated lists that can silently drift.
|
|
- A ratchet must be monotonic and must not be weakenable by the PR it checks.
|
|
- Require focused tests for both detection and false-positive behavior.
|
|
- Do not duplicate GitHub issue tracking, CI status, or another advisor's
|
|
responsibility.
|
|
|
|
- path: "tools/{pr-review-advisor,e2e-advisor}/**"
|
|
instructions: *guardrail
|
|
|
|
knowledge_base:
|
|
code_guidelines:
|
|
enabled: true
|
|
|
|
chat:
|
|
auto_reply: true
|