NemoClaw/.coderabbit.yaml
Carlos Villela 8120223922
test(e2e): retire legacy shell lanes (#5756)
<!-- 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>
2026-06-29 22:32:24 -05:00

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