perf(test): remove mocked recovery waits

[cv]

Summary

This PR removes production-length recovery waits from four fully mocked integration scenarios by setting the existing delay controls to zero within those tests. Production defaults and the dedicated forward-release timing contracts remain unchanged; on Node 22, the focused pair fell from 16.15s to 1.65s wall time.

Related Issue

Refs #6245

Changes

• Preserve mocked Hermes supervisor retries without sleeping between attempts.
• Skip the production forward-visibility window in three scenarios whose outcomes are fully controlled by mocks.
• Restore stubbed environment values after the forward-failure tests.
• Reduce the four affected assertions from 15.028s combined to 22ms while retaining the explicit 1000ms delayed-release and 150ms fail-closed tests.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: Test-only timing cleanup; production defaults, configuration, and user-visible behavior are unchanged.
• 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:
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Node 22.16.0; npx --yes --package=node@22.16.0 node ./node_modules/vitest/vitest.mjs run --project integration test/process-recovery.test.ts test/process-recovery-forward-failure.test.ts --reporter=verbose; 29/29 passed in 1.06s Vitest time (1.65s wall).
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

• Tests
  • Updated recovery test coverage to run faster and more reliably by removing unnecessary wait times in mocked scenarios.
  • Improved test isolation by resetting environment stubs after each test.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e88ab757-8eed-4a39-8a0a-78a2d8976935

📥 Commits

Reviewing files that changed from the base of the PR and between 92f883a and 37f04ef.

📒 Files selected for processing (2)

• test/process-recovery-forward-failure.test.ts
• test/process-recovery.test.ts

---

📝 Walkthrough

Walkthrough

Test files for process recovery now stub environment variables (NEMOCLAW_FORWARD_RECOVERY_WAIT_MS, NEMOCLAW_GATEWAY_RECOVERY_POLL_INTERVAL_SECONDS) to "0" to eliminate production timing delays during mocked test scenarios, and afterEach now calls vi.unstubAllEnvs() for cleanup.

Changes

Recovery test timing stubs

Layer / File(s)	Summary
Forward-failure test env stubbing and cleanup test/process-recovery-forward-failure.test.ts	Adds vi.unstubAllEnvs() to afterEach and stubs NEMOCLAW_FORWARD_RECOVERY_WAIT_MS to "0" in a secondary-forward recovery test.
Process-recovery test env stubbing test/process-recovery.test.ts	Stubs NEMOCLAW_FORWARD_RECOVERY_WAIT_MS and NEMOCLAW_GATEWAY_RECOVERY_POLL_INTERVAL_SECONDS to "0" across three Hermes-forward and forward-recovery test scenarios.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

• NVIDIA/NemoClaw#4442: Related to the same checkAndRecoverSandboxProcesses recovery behavior these tests exercise, including Hermes forwarding logic.

Suggested labels: chore

Suggested reviewers: kjw3

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: removing mocked recovery waits in tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/perf-recovery-test-waits

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No E2E is recommended because this PR is tests-only and only adjusts mocked unit test environment/timing behavior for process recovery; it cannot affect real runtime or user flows.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

• None. Only non-E2E test files outside test/e2e changed; these changes do not affect the NemoClaw E2E target workflow, registry, fixtures, live support, or target behavior.

Optional E2E targets

• None.

Relevant changed files

• None.

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/perf-recovery-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/perf-recovery-... 37f04ef	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the codex/perf-recovery-... branch is 74%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/perf-recovery-... 37f04ef	+/-
src/lib/shields...nsition-lock.ts	—	87%	—
src/lib/onboard/preflight.ts	—	83%	—
src/lib/actions...all/run-plan.ts	—	81%	—
src/lib/state/o...oard-session.ts	—	80%	—
src/lib/state/sandbox.ts	—	75%	—
src/lib/onboard...er-gpu-patch.ts	—	70%	—
src/lib/shields/index.ts	—	67%	—
src/lib/policy/index.ts	—	67%	—
src/lib/actions...licy-channel.ts	—	63%	—
src/lib/onboard.ts	—	28%	—

---

_{Updated July 06, 2026 21:59 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 1 suggestion · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-T1 Add or justify test follow-up: Added vi.unstubAllEnvs() to afterEach for test isolation
• PRA-1 In-scope improvement: Added vi.unstubAllEnvs() to afterEach for test isolation in test/process-recovery-forward-failure.test.ts:1

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Improvement	tests	test/process-recovery-forward-failure.test.ts:1	No action needed; this is a good practice addition.

Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

• None.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Added vi.unstubAllEnvs() to afterEach for test isolation

• Location: test/process-recovery-forward-failure.test.ts:1
• Category: tests
• Problem: The forward-failure test file now restores all stubbed environment variables after each test, preventing cross-test leakage from the new NEMOCLAW_FORWARD_RECOVERY_WAIT_MS=0 stubs.
• Impact: Improves test reliability and isolation; no runtime behavior change.
• Suggested action: No action needed; this is a good practice addition.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check test/process-recovery-forward-failure.test.ts afterEach block contains vi.unstubAllEnvs() call.
• Missing regression test: Existing test suite passes; isolation is verified by test run success.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Diff shows +vi.unstubAllEnvs() in afterEach; 29/29 tests pass.

Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-1 native (test/process-recovery-forward-failure.test.ts:1): Manual env restoration in individual tests (none present)
  • Replacement: Single vi.unstubAllEnvs() call in afterEach
  • Net: 1 lines
  • Safety boundary: Test isolation only; no production code affected

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Added vi.unstubAllEnvs() to afterEach for test isolation — No action needed; this is a good practice addition.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Added vi.unstubAllEnvs() to afterEach for test isolation

• Location: test/process-recovery-forward-failure.test.ts:1
• Category: tests
• Problem: The forward-failure test file now restores all stubbed environment variables after each test, preventing cross-test leakage from the new NEMOCLAW_FORWARD_RECOVERY_WAIT_MS=0 stubs.
• Impact: Improves test reliability and isolation; no runtime behavior change.
• Suggested action: No action needed; this is a good practice addition.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check test/process-recovery-forward-failure.test.ts afterEach block contains vi.unstubAllEnvs() call.
• Missing regression test: Existing test suite passes; isolation is verified by test run success.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Diff shows +vi.unstubAllEnvs() in afterEach; 29/29 tests pass.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 1 test follow-up
Top item: No actionable code-review findings

Action checklist

• PRA-T1 Add or justify test follow-up: Acceptance clause

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Acceptance clause — Reduce the four affected assertions from 15.028s combined to 22ms while retaining the explicit 1000ms delayed-release and 150ms fail-closed tests. — add test evidence or identify existing coverage. The diff implements the timing-control mechanism by zeroing existing wait/poll env knobs in four mocked scenarios, and existing nearby tests still explicitly set NEMOCLAW_FORWARD_RECOVERY_WAIT_MS to "1000" for delayed release and "150" for fail-closed listener coverage. The exact wall-time claim was not verified in this read-only review.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.