Skip to content

ci(e2e): restore Anthropic switch variants - #5947

Merged
cv merged 13 commits into
mainfrom
codex/e2e-anthropic-switch-matrix
Jun 29, 2026
Merged

ci(e2e): restore Anthropic switch variants#5947
cv merged 13 commits into
mainfrom
codex/e2e-anthropic-switch-matrix

Conversation

@cv

@cv cv commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Restores the two v0.0.69 Anthropic inference-switch workflow variants as distinct Vitest matrix executions. The existing OpenClaw and Hermes live tests already implement the compatible Anthropic provider, mock protocol, route/config mutation, inference.local, and agent assertions; this PR makes that mode unavoidable in default and selected CI runs.

Related Issue

Refs #5919

Changes

  • run hosted-compatible and Anthropic-compatible legs for both inference-switch jobs with fail-fast: false
  • preserve the old Anthropic provider, API, model, mock, agent, and sandbox-name mode values
  • separate artifacts by agent and mode so each execution has reviewable evidence
  • limit Docker credentials to the OpenClaw live-test window and remove them before artifact upload
  • add a focused workflow-boundary ratchet with mutations for removed modes, hard-coded mode mappings, and credential-cleanup ordering
  • restore the v0.0.69 Hermes gateway-reset lifecycle boundary before isolated install attempts
  • share the v0.0.69 inference-switch retry policy across Hermes and OpenClaw: verified attempts first, --no-verify only after bounded transient failures, then mandatory live route/config/inference/agent assertions

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: CI execution-mode restoration and test-harness parity; no production CLI, configuration, or user-workflow change. Required docs-writer reassessment after the final fix confirmed no docs changes.
  • 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: pending independent human review; CodeQL, CodeRabbit, both PR advisors, the credential scan, and exact live matrix evidence are recorded below.
  • 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
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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)

Verification evidence:

  • e2e-vitest-support: 57 files / 417 tests passed with 16 workers.
  • Focused inference-switch support suite: 14 tests passed, covering workflow mode ratchets, Hermes command/lifecycle wiring, verified transient retries, fallback only after retry exhaustion, no fallback for non-transient failures, and attempt-count validation.
  • npm run typecheck, npm run typecheck:cli, Biome on changed TypeScript, and npm run test:projects:check passed; Vitest reports 903 files across 8 disjoint projects.
  • Normal commit and push hooks passed, including the broad CLI coverage/ratchet lane, source-shape and test-size budgets, repository checks, secret scanning, commitlint, and CLI TypeScript checks.
  • All non-skipped PR checks pass at final head 9bebd96bba7c3748ad61ff60d2690b0cd7df90dc.
  • Exact-head live run 28383771048 passed all four cells: Hermes hosted, Hermes Anthropic-compatible, OpenClaw hosted, and OpenClaw Anthropic-compatible.
  • Downloaded artifacts contain four distinct agent/mode roots. Both OpenClaw scenario-result.json files report passed with route, config, registry/session, inference.local, and agent PONG assertions true. Both Hermes modes have zero-exit route, config/hash, inference.local, API-chat, and primary destroy results; both chat boundaries returned PONG.
  • Hermes Anthropic exhausted three transient host-side endpoint-verification attempts, used the bounded --no-verify fallback, then passed the independent OpenShell route, Hermes config/hash, registry/session, sandbox inference.local, and Hermes API checks. The direct Anthropic probe used the frozen max_tokens: 32 payload and returned PONG on its first bounded live attempt; the Hermes API boundary also returned PONG. This is the same safety boundary as the frozen v0.0.69 shared shell helper.
  • Both OpenClaw jobs completed isolated Docker-auth setup, authentication, live test, Docker-auth cleanup, then artifact upload in that order.
  • All four cleanup reports have zero failures; no command result timed out; the downloaded artifacts contain no NVIDIA/GitHub/Slack/AWS/OpenAI credential-shaped matches.
  • Required documentation-writer assessment, rerun after the final shared-helper fix, found no user-facing docs changes.

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

Summary by CodeRabbit

  • New Features

    • Expanded inference-switch e2e scenarios to run in both hosted and Anthropic-compatible modes, with mode-aware artifact naming/paths.
  • Bug Fixes

    • Improved e2e reliability by adding shared retry orchestration for inference-switch and Hermes PONG checks, retrying transient failures and applying --no-verify only when the retry budget is exhausted.
    • Refined Hermes and OpenClaw lifecycle orchestration (setup, install, and cleanup) for more consistent sandbox/gateway handling.
  • Tests / Validation

    • Added end-to-end retry coverage and strengthened workflow boundary validation, including matrix constraints and mode-scoped Docker auth cleanup behavior.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 28, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Inference-switch CI jobs for hermes and openclaw now run as hosted and anthropic mode matrices. Shared retry logic moves into a fixture, hermes helpers add mode-aware orchestration, and workflow-boundary validation now checks the updated workflow shape.

Changes

Inference Switch Mode-Aware Matrix and Shared Retry

Layer / File(s) Summary
CI workflow mode-aware matrix conversion
.github/workflows/e2e-vitest-scenarios.yaml
Converts both inference-switch jobs to hosted and anthropic matrices, wiring matrix mode into environment values, artifact names, and OpenClaw Docker cleanup.
Shared inference-set retry fixture
test/e2e-scenario/fixtures/inference-switch-retry.ts, test/e2e-scenario/support-tests/inference-switch-retry.test.ts
Introduces shared retry parsing, transient-failure detection, and retry execution, plus tests for retry behavior and attempt-count validation.
Hermes helper orchestration
test/e2e-scenario/live/hermes-inference-switch-helpers.ts, test/e2e-scenario/live/hermes-inference-switch.test.ts, test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts
Adds hosted model and gateway helpers, updates Hermes environment and lifecycle commands, adds Hermes inference-set retry orchestration, and updates live and command-shape tests.
OpenClaw live test retry refactor
test/e2e-scenario/live/openclaw-inference-switch.test.ts
Replaces local OpenClaw retry parsing and loop logic with the shared retry helper and updates the live scenario call site.
Workflow boundary validator for inference-switch jobs
tools/e2e-scenarios/inference-switch-workflow-boundary.mts, tools/e2e-scenarios/workflow-boundary.mts, test/e2e-scenario/support-tests/inference-switch-workflow-boundary.test.ts
Adds inference-switch workflow shape validation, wires it into the central boundary checker, and covers the expected and invalid workflow configurations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

area: inference, integration: hermes, bug-fix

Suggested reviewers

  • jyaunches
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: restoring separate Anthropic switch workflow variants in e2e CI.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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/e2e-anthropic-switch-matrix

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

@github-code-quality

github-code-quality Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/e2e-anthropic-... 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/e2e-anthropic-... 9bebd96 +/-
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/e2e-anthropic-... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/e2e-anthropic-... 9bebd96 +/-
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/actions...dbox/rebuild.ts 74%
src/lib/state/sandbox.ts 72%
src/lib/shields/index.ts 70%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/policy/index.ts 52%
src/lib/onboard.ts 20%

Updated June 29, 2026 15:39 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: hermes-inference-switch-vitest, openclaw-inference-switch-vitest
Optional E2E: bedrock-runtime-compatible-anthropic-vitest

Dispatch hint: hermes-inference-switch-vitest,openclaw-inference-switch-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • hermes-inference-switch-vitest (high): This PR directly changes the Hermes inference-switch live scenario and the corresponding workflow job matrix/env. Run the job to validate both hosted and Anthropic-compatible inference-switch modes, route/config updates, and Hermes live chat probes.
  • openclaw-inference-switch-vitest (high): This PR directly changes the OpenClaw inference-switch live scenario and the corresponding workflow job matrix/env/Docker auth handling. Run the job to validate both hosted and Anthropic-compatible switch modes, OpenShell route state, OpenClaw config/registry state, inference.local, and the live OpenClaw agent turn.

Optional E2E

  • bedrock-runtime-compatible-anthropic-vitest (high): Optional adjacent confidence for Anthropic-compatible runtime contracts across Hermes and OpenClaw. The PR adds Anthropic-compatible inference-switch modes, but it does not modify Bedrock runtime integration directly, so this is not merge-blocking.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: hermes-inference-switch-vitest,openclaw-inference-switch-vitest

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: hermes-inference-switch-vitest, openclaw-inference-switch-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-inference-switch-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • hermes-inference-switch-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/hermes-inference-switch.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-inference-switch-vitest
  • openclaw-inference-switch-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/openclaw-inference-switch.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-inference-switch-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/fixtures/inference-switch-retry.ts
  • test/e2e-scenario/live/hermes-inference-switch-helpers.ts
  • test/e2e-scenario/live/hermes-inference-switch.test.ts
  • test/e2e-scenario/live/openclaw-inference-switch.test.ts
  • test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts
  • test/e2e-scenario/support-tests/inference-switch-retry.test.ts
  • test/e2e-scenario/support-tests/inference-switch-workflow-boundary.test.ts
  • tools/e2e-scenarios/inference-switch-workflow-boundary.mts
  • tools/e2e-scenarios/workflow-boundary.mts

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Blocked

Merge posture: Do not merge until addressed
Primary next action: Fix PRA-6: installHermes --fresh workaround lacks TODO linking to root-cause fix; then add or justify PRA-T1.
Open items: 3 required · 17 warnings · 11 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 18 still apply · 7 new items found

Action checklist

  • PRA-6 Fix: installHermes --fresh workaround lacks TODO linking to root-cause fix in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • PRA-7 Fix: --fresh flag masks onboarding state leak without regression test in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • PRA-8 Fix: --fresh workaround lacks source-of-truth justification comment in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • PRA-1 Resolve or justify: Source-of-truth review needed: inferenceSetAttemptCount fallback=3
  • PRA-2 Resolve or justify: Source-of-truth review needed: parseHermesModelBlock silent catch
  • PRA-3 Resolve or justify: Source-of-truth review needed: TRANSIENT_INFERENCE_SET_FAILURE regex workaround
  • PRA-4 Resolve or justify: Source-of-truth review needed: agentReplyContainsToken strict equality
  • PRA-5 Resolve or justify: Source-of-truth review needed: parseOpenClawAgentText regex JSON fallback
  • PRA-9 Resolve or justify: TRANSIENT_INFERENCE_SET_FAILURE regex over-matches with bare 'temporar' prefix in test/e2e-scenario/fixtures/inference-switch-retry.ts:7
  • PRA-10 Resolve or justify: mockAnthropicEndpointUrl accepts arbitrary host without allowlist (SSRF risk) in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40
  • PRA-11 Resolve or justify: env() sets both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL without comment in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62
  • PRA-12 Resolve or justify: isDeepStrictEqual matrix ratchet lacks documentation in tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84
  • PRA-13 Resolve or justify: New validator module duplicates workflow-boundary.mts validation patterns in tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: inferenceSetAttemptCount fallback=3
  • PRA-T7 Add or justify test follow-up: parseHermesModelBlock silent catch
  • PRA-T8 Add or justify test follow-up: TRANSIENT_INFERENCE_SET_FAILURE regex workaround
  • PRA-19 In-scope improvement: DEFAULT_HOSTED_INFERENCE_MODEL import only used as fallback constant in tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45
  • PRA-20 In-scope improvement: inferenceSetAttemptCount env configurability may be YAGNI in test/e2e-scenario/fixtures/inference-switch-retry.ts:9
  • PRA-21 In-scope improvement: hostedInstallModel() single-use helper adds indirection in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:50
  • PRA-22 In-scope improvement: Retry fixture tests missing backoff progression and edge case coverage in test/e2e-scenario/support-tests/inference-switch-retry.test.ts:1
  • PRA-23 In-scope improvement: workflow-boundary.mts imports inference-switch validator creating coupling in tools/e2e-scenarios/workflow-boundary.mts:9
  • PRA-24 In-scope improvement: No behavioral test for installHermes --fresh flag in test/e2e-scenario/support-tests/inference-switch-retry.test.ts:1
  • PRA-25 In-scope improvement: Support test names could be more behavior-specific in test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:100
  • PRA-28 In-scope improvement: DEFAULT_HOSTED_INFERENCE_MODEL single-use import in test/e2e-scenario/live/hermes-inference-switch-helpers.ts:18
  • PRA-29 In-scope improvement: DEFAULT_HOSTED_INFERENCE_MODEL single-use import in support test in test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:10
  • PRA-30 In-scope improvement: DEFAULT_WORKFLOW_PATH unnecessary config knob in tools/e2e-scenarios/inference-switch-workflow-boundary.mts:11

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-4 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-5 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-6 Required architecture test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 Add a TODO comment at line 328 referencing a GitHub issue tracking the onboarding cleanup fix, or document why --fresh is the correct permanent solution.
PRA-7 Required correctness test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 Add a test that install.sh succeeds without --fresh when onboarding state is clean, proving the root cause is fixed. Alternatively, fix onboarding cleanup at source in install.sh.
PRA-8 Required correctness test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 Add a code comment at line 328 with: the invalid state, source boundary, source-fix constraint, regression test proving source fixed, and removal condition.
PRA-9 Resolve/justify architecture test/e2e-scenario/fixtures/inference-switch-retry.ts:7 Tighten regex to provider-specific patterns: 'temporarily unavailable', 'temporary failure', 'temporary error'. Remove bare 'temporar' prefix.
PRA-10 Resolve/justify security test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40 Add allowlist validation (e.g., only '127.0.0.1', 'host.openshell.internal', 'localhost') or document as test-only trusted input with a comment explaining the trust boundary.
PRA-11 Resolve/justify architecture test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62 Add a brief comment explaining why both are set (matches hosted-inference.ts contract), or consider a helper to set both atomically.
PRA-12 Resolve/justify architecture tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84 Add a code comment at expectedModes() explaining that adding modes requires updating this function and the validation, or consider a 'must contain at least these two modes' approach using subset matching.
PRA-13 Resolve/justify architecture tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1 Merge validateInferenceSwitchWorkflow into workflow-boundary.mts validateE2eVitestScenariosWorkflowBoundary. Keep validation DRY and avoid circular-like dependency.
PRA-14 Resolve/justify architecture test/e2e-scenario/live/openclaw-inference-switch.test.ts:708 Change to substring/includes check (normalizedReply.includes(normalizedExpected)) or document why exact match is required for this test's contract.
PRA-15 Resolve/justify architecture test/e2e-scenario/live/openclaw-inference-switch.test.ts:717 Replace with a proper streaming JSON parser or require the agent output to be strict JSON. If fallback is necessary, add a comment explaining why the source cannot produce clean JSON and what regression test proves it cannot regress.
PRA-16 Resolve/justify architecture test/e2e-scenario/fixtures/inference-switch-retry.ts:7 Add a comment at the regex definition explaining: what invalid state is handled (transient network/provider errors), where that state is created (provider API), why the source cannot be fixed in this PR (external API), what regression test proves the source cannot regress, and when the workaround can be removed.
PRA-17 Resolve/justify security .github/workflows/e2e-vitest-scenarios.yaml:906 Verify that NVIDIA_INFERENCE_API_KEY secret is appropriately scoped and not over-privileged for the mock anthropic mode. Consider if a separate mock credential would be cleaner.
PRA-18 Resolve/justify correctness .github/workflows/e2e-vitest-scenarios.yaml:4700 Verify that docker CLI is available before 'Configure isolated Docker auth directory' step runs. The runner should have docker pre-installed, but explicit check would be safer.
PRA-19 Improvement architecture tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45 Replace import with local constant: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3'; in helpers.ts. Also update support test import.
PRA-20 Improvement architecture test/e2e-scenario/fixtures/inference-switch-retry.ts:9 Consider inlining the constant 3 at call sites or removing env configurability if not used in CI.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-6 Required — installHermes --fresh workaround lacks TODO linking to root-cause fix

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: architecture
  • Problem: installHermes() unconditionally passes --fresh flag to install.sh to discard failed onboarding state, but no TODO comment references a GitHub issue tracking the onboarding cleanup root-cause fix. This was flagged in the prior review (PRA-5) and remains unaddressed.
  • Impact: If onboarding failure mode regresses or changes, --fresh may mask the symptom without fixing the underlying state leak. No visibility into whether the root cause is being tracked.
  • Required action: Add a TODO comment at line 328 referencing a GitHub issue tracking the onboarding cleanup fix, or document why --fresh is the correct permanent solution.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 — installHermes passes ['install.sh', '--non-interactive', '--fresh', '--yes-i-accept-third-party-software']; check for TODO comment referencing follow-up issue.
  • Missing regression test: GitHub issue tracking the root-cause fix with acceptance criteria for clean onboarding state.
  • Done when: The required change is committed and verification passes: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 — installHermes passes ['install.sh', '--non-interactive', '--fresh', '--yes-i-accept-third-party-software']; check for TODO comment referencing follow-up issue.
  • Evidence: installHermes() at helpers.ts:328 passes --fresh; support test at hermes-inference-switch-command-shape.test.ts:90 only verifies argv shape

PRA-7 Required — --fresh flag masks onboarding state leak without regression test

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: correctness
  • Problem: The --fresh flag masks potential onboarding state leak without a regression test proving install.sh succeeds without --fresh when onboarding state is clean. Flagged in prior review (PRA-6) and unaddressed.
  • Impact: Cannot verify the root cause is actually fixed; the workaround may become permanent without detection.
  • Required action: Add a test that install.sh succeeds without --fresh when onboarding state is clean, proving the root cause is fixed. Alternatively, fix onboarding cleanup at source in install.sh.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if any test verifies install.sh succeeds without --fresh on a clean sandbox state.
  • Missing regression test: Test that install.sh --non-interactive (without --fresh) succeeds on a pristine sandbox, demonstrating the root cause is fixed.
  • Done when: The required change is committed and verification passes: Check if any test verifies install.sh succeeds without --fresh on a clean sandbox state.
  • Evidence: No existing test covers install.sh without --fresh on clean state

PRA-8 Required — --fresh workaround lacks source-of-truth justification comment

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: correctness
  • Problem: The --fresh workaround lacks a source-of-truth justification comment explaining: the invalid state (failed onboarding leaves sandbox unusable), source boundary (install.sh onboarding logic), source-fix constraint (requires install.sh change outside this PR), regression test proving source fixed, and removal condition. Flagged in prior review (PRA-7) and unaddressed.
  • Impact: Without documentation, the workaround cannot be evaluated for removal when the root cause is fixed. Maintainers may not know it exists or why.
  • Required action: Add a code comment at line 328 with: the invalid state, source boundary, source-fix constraint, regression test proving source fixed, and removal condition.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 and surrounding lines for source-of-truth justification comment.
  • Missing regression test: Source-of-truth comment documenting all five elements at the workaround site.
  • Done when: The required change is committed and verification passes: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 and surrounding lines for source-of-truth justification comment.
  • Evidence: No comment at installHermes() call site explaining the workaround rationale
Review findings by urgency: 3 required fixes, 17 items to resolve/justify, 11 in-scope improvements

⚠️ 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.

PRA-1 Resolve/justify — Source-of-truth review needed: inferenceSetAttemptCount fallback=3

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test for env var being set in CI
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:9-15

PRA-2 Resolve/justify — Source-of-truth review needed: parseHermesModelBlock silent catch

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test with malformed JSON
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142

PRA-3 Resolve/justify — Source-of-truth review needed: TRANSIENT_INFERENCE_SET_FAILURE regex workaround

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test documenting which provider errors are transient
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:7

PRA-4 Resolve/justify — Source-of-truth review needed: agentReplyContainsToken strict equality

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Support test 'openclaw-inference-switch agent reply matching tolerates wrapped PONG' exists but expects strict equality
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts:708

PRA-5 Resolve/justify — Source-of-truth review needed: parseOpenClawAgentText regex JSON fallback

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Support test exists but uses regex fallback behavior
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts:717

PRA-9 Resolve/justify — TRANSIENT_INFERENCE_SET_FAILURE regex over-matches with bare 'temporar' prefix

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:7
  • Category: architecture
  • Problem: TRANSIENT_INFERENCE_SET_FAILURE regex includes bare 'temporar' prefix which over-matches (e.g., 'temporary', 'temporarily' but also could match unrelated words containing 'temporar'). Flagged in prior review (PRA-10/PRA-14) and unaddressed.
  • Impact: May incorrectly classify non-transient errors as transient, causing inappropriate retries and --no-verify fallback.
  • Recommended action: Tighten regex to provider-specific patterns: 'temporarily unavailable', 'temporary failure', 'temporary error'. Remove bare 'temporar' prefix.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex contains '|temporar' without word boundary or provider context.
  • Missing regression test: Test that 'temporarily unavailable' matches but 'temporar' alone does not.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex contains '|temporar' without word boundary or provider context.
  • Evidence: Regex: /timed? out|timeout|ETIMEDOUT|ECONNRESET|EAI_AGAIN|ENOTFOUND|failed to connect|error sending request|failed to verify inference endpoint|502|503|504|temporar/iu

PRA-10 Resolve/justify — mockAnthropicEndpointUrl accepts arbitrary host without allowlist (SSRF risk)

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40
  • Category: security
  • Problem: mockAnthropicEndpointUrl() accepts arbitrary host via NEMOCLAW_SWITCH_MOCK_HOST env var without allowlist validation. In a test context this is lower risk, but could enable SSRF if env var is attacker-controlled. Flagged in prior review (PRA-13) and unaddressed.
  • Impact: If NEMOCLAW_SWITCH_MOCK_HOST is set to an internal or external address, the mock server could be reached at that address, potentially enabling SSRF.
  • Recommended action: Add allowlist validation (e.g., only '127.0.0.1', 'host.openshell.internal', 'localhost') or document as test-only trusted input with a comment explaining the trust boundary.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40 — function uses runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal' without validation.
  • Missing regression test: Test that invalid host values are rejected or sanitized.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40 — function uses runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal' without validation.
  • Evidence: mockAnthropicEndpointUrl(port, runtimeEnv) returns `http://${runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal'}:${port}`

PRA-11 Resolve/justify — env() sets both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL without comment

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62
  • Category: architecture
  • Problem: env() sets both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL to the same value (hostedInstallModel()) without a comment explaining why both are needed. Flagged in prior review (PRA-12) and unaddressed.
  • Impact: Future maintainers may not understand the dual assignment and could break the hosted-inference.ts contract.
  • Recommended action: Add a brief comment explaining why both are set (matches hosted-inference.ts contract), or consider a helper to set both atomically.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62-63 — both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL assigned hostedInstallModel() with no explanatory comment.
  • Missing regression test: Documentation comment at the assignment site.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62-63 — both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL assigned hostedInstallModel() with no explanatory comment.
  • Evidence: Lines 62-63: NEMOCLAW_MODEL: hostedInstallModel(), NEMOCLAW_COMPAT_MODEL: hostedInstallModel()

PRA-12 Resolve/justify — isDeepStrictEqual matrix ratchet lacks documentation

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84
  • Category: architecture
  • Problem: validateJob uses isDeepStrictEqual to compare the entire matrix.include array against expectedModes(). This creates a ratchet: adding new modes requires updating both the expectedModes() function and the validation logic, with no comment explaining this coupling. Flagged in prior review (PRA-11) and unaddressed.
  • Impact: Adding a new mode (e.g., a third provider) will fail validation silently unless both places are updated. No guidance for maintainers.
  • Recommended action: Add a code comment at expectedModes() explaining that adding modes requires updating this function and the validation, or consider a 'must contain at least these two modes' approach using subset matching.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84 — isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)) with no comment about the ratchet.
  • Missing regression test: Comment documenting the ratchet, or refactor to subset validation.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84 — isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)) with no comment about the ratchet.
  • Evidence: Line 84: if (!isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)))

PRA-13 Resolve/justify — New validator module duplicates workflow-boundary.mts validation patterns

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1
  • Category: architecture
  • Problem: New validator module inference-switch-workflow-boundary.mts duplicates validation patterns from workflow-boundary.mts (permission checks, step ordering, artifact naming) rather than merging into the existing validateE2eVitestScenariosWorkflowBoundary. Creates cross-module coupling (workflow-boundary.mts imports it). Flagged in prior review (PRA-15, PRA-19) and unaddressed.
  • Impact: Duplicated validation logic increases maintenance burden and risk of divergence. Circular-like dependency between modules.
  • Recommended action: Merge validateInferenceSwitchWorkflow into workflow-boundary.mts validateE2eVitestScenariosWorkflowBoundary. Keep validation DRY and avoid circular-like dependency.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare tools/e2e-scenarios/inference-switch-workflow-boundary.mts and workflow-boundary.mts for duplicated patterns (permissions, fail-fast, artifact naming, step ordering).
  • Missing regression test: Single consolidated validator with shared helper functions.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare tools/e2e-scenarios/inference-switch-workflow-boundary.mts and workflow-boundary.mts for duplicated patterns (permissions, fail-fast, artifact naming, step ordering).
  • Evidence: workflow-boundary.mts:9 imports and :7474 calls validateInferenceSwitchWorkflowBoundary; both modules validate permissions.contents === 'read', fail-fast === false, artifact naming

PRA-14 Resolve/justify — agentReplyContainsToken uses strict equality causing false negatives

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:708
  • Category: architecture
  • Problem: agentReplyContainsToken uses strict equality (normalizedReply === normalizedExpected) causing false negatives when agent output contains extra text around 'PONG'. Flagged in prior review (PRA-1/PRA-8) and unaddressed.
  • Impact: Valid agent responses containing 'PONG' with additional context (e.g., 'The answer is PONG') will fail the test, reducing test reliability.
  • Recommended action: Change to substring/includes check (normalizedReply.includes(normalizedExpected)) or document why exact match is required for this test's contract.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:708 — function returns normalizedReply === normalizedExpected.
  • Missing regression test: Test that 'the answer is PONG' passes (currently fails with strict equality).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:708 — function returns normalizedReply === normalizedExpected.
  • Evidence: Line 708: return normalizedReply === normalizedExpected;

PRA-15 Resolve/justify — parseOpenClawAgentText uses fragile regex-based JSON fallback

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:717
  • Category: architecture
  • Problem: parseOpenClawAgentText uses fragile regex-based JSON fallback (/{/g) to find JSON envelopes in agent output. This is a localized workaround for malformed agent output. Flagged in prior review (PRA-2/PRA-9) and unaddressed.
  • Impact: Fragile parsing may break with format changes, miss valid JSON, or accept invalid JSON. No clear contract for agent output format.
  • Recommended action: Replace with a proper streaming JSON parser or require the agent output to be strict JSON. If fallback is necessary, add a comment explaining why the source cannot produce clean JSON and what regression test proves it cannot regress.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:717 — regex /{/g used to find JSON start positions.
  • Missing regression test: Source-of-truth comment documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:717 — regex /{/g used to find JSON start positions.
  • Evidence: Line 717: const decoder = new RegExp('{', 'g'); while ((match = decoder.exec(raw)) !== null)

PRA-16 Resolve/justify — TRANSIENT_INFERENCE_SET_FAILURE regex lacks source-of-truth justification

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:7
  • Category: architecture
  • Problem: TRANSIENT_INFERENCE_SET_FAILURE regex is a localized workaround for transient provider failures without source-of-truth justification. Flagged in prior review (PRA-3) and unaddressed.
  • Impact: Cannot evaluate when this workaround can be removed or if the provider API has improved. No visibility into what specific transient errors are expected.
  • Recommended action: Add a comment at the regex definition explaining: what invalid state is handled (transient network/provider errors), where that state is created (provider API), why the source cannot be fixed in this PR (external API), what regression test proves the source cannot regress, and when the workaround can be removed.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex definition with no source-of-truth comment.
  • Missing regression test: Source-of-truth comment at regex definition site.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex definition with no source-of-truth comment.
  • Evidence: Line 7: const TRANSIENT_INFERENCE_SET_FAILURE = /timed? out|timeout|...|temporar/iu;

PRA-17 Resolve/justify — NVIDIA_INFERENCE_API_KEY secret used for mock Anthropic mode

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:906
  • Category: security
  • Problem: The hermes-inference-switch-vitest and openclaw-inference-switch-vitest jobs use matrix strategy with mode-specific sandbox names and artifact paths, but the workflow still uses NVIDIA_INFERENCE_API_KEY secret for both modes. The anthropic mode uses a mock provider but still requires the NVIDIA secret for onboarding.
  • Impact: Over-privileged secret usage; mock mode doesn't need real NVIDIA credentials. Could expose production credentials in test logs if misconfigured.
  • Recommended action: Verify that NVIDIA_INFERENCE_API_KEY secret is appropriately scoped and not over-privileged for the mock anthropic mode. Consider if a separate mock credential would be cleaner.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:906-940 — both matrix modes use NVIDIA_INFERENCE_API_KEY secret.
  • Missing regression test: Secret scope documentation in workflow comments.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:906-940 — both matrix modes use NVIDIA_INFERENCE_API_KEY secret.
  • Evidence: Both matrix modes in hermes-inference-switch-vitest and openclaw-inference-switch-vitest jobs have env: NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }}

PRA-18 Resolve/justify — OpenClaw job step reordering may break Docker login

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:4700
  • Category: correctness
  • Problem: openclaw-inference-switch-vitest job reorders steps: 'Configure isolated Docker auth directory' and 'Authenticate to Docker Hub' now run before 'Set up Node'/'Install root dependencies'/'Build CLI'. This changes the execution order — Docker auth config now runs before Node setup. Need to verify this doesn't break Docker login (which needs docker CLI).
  • Impact: If docker CLI is not available at the Docker auth step, the login will fail, breaking the OpenClaw test.
  • Recommended action: Verify that docker CLI is available before 'Configure isolated Docker auth directory' step runs. The runner should have docker pre-installed, but explicit check would be safer.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:4700-4760 — step order changed; docker login step runs after 'Authenticate to Docker Hub' but before Node setup.
  • Missing regression test: CI verification that docker CLI is available at Docker auth step.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:4700-4760 — step order changed; docker login step runs after 'Authenticate to Docker Hub' but before Node setup.
  • Evidence: Steps 4700-4760: Configure isolated Docker auth directory -> Authenticate to Docker Hub -> Set up Node -> Install root dependencies -> Build CLI

PRA-26 Resolve/justify — inferenceSetAttemptCount fallback lacks source-of-truth justification

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:9
  • Category: architecture
  • Problem: inferenceSetAttemptCount() provides fallback=3 for undefined env var — a localized tolerance for missing configuration. Source-of-truth questions unanswered.
  • Impact: Cannot evaluate when this fallback can be removed or if the env var should be required.
  • Recommended action: Add source-of-truth comment at function definition, or remove env configurability if YAGNI.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check if NEMOCLAW_SWITCH_SET_ATTEMPTS is set in any CI workflow.
  • Missing regression test: Source-of-truth comment or removal of env configurability.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check if NEMOCLAW_SWITCH_SET_ATTEMPTS is set in any CI workflow.
  • Evidence: Line 9: export function inferenceSetAttemptCount(raw: string | undefined, fallback = 3): number { if (raw === undefined) return fallback; }

PRA-27 Resolve/justify — Silent catch in parseHermesModelBlock lacks justification

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142
  • Category: architecture
  • Problem: Silent catch {} in parseHermesModelBlock's try-catch when extracting chat content — a localized tolerance for malformed JSON. Source-of-truth questions unanswered.
  • Impact: Malformed chat responses are silently ignored, potentially masking real failures in PONG detection.
  • Recommended action: Add comment explaining why silent catch is acceptable here, or improve parsing to be more explicit about expected format.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142 — try { pong = /PONG/iu.test(chatContent(last.stdout)); } catch {}
  • Missing regression test: Source-of-truth comment or improved error handling.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142 — try { pong = /PONG/iu.test(chatContent(last.stdout)); } catch {}.
  • Evidence: Line 142: try { pong = /PONG/iu.test(chatContent(last.stdout)); } catch {}

💡 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-19 Improvement — DEFAULT_HOSTED_INFERENCE_MODEL import only used as fallback constant

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45
  • Category: architecture
  • Problem: DEFAULT_HOSTED_INFERENCE_MODEL imported from ../fixtures/hosted-inference.ts but only used as fallback constant in hostedInstallModel(). Could be inlined as local constant to reduce cross-module dependency. Flagged in prior review (PRA-16) and unaddressed.
  • Impact: Unnecessary cross-module dependency for a single constant value.
  • Suggested action: Replace import with local constant: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3'; in helpers.ts. Also update support test import.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45 — import used only in hostedInstallModel fallback.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Import at line 45, used only in hostedInstallModel() at line 58

PRA-20 Improvement — inferenceSetAttemptCount env configurability may be YAGNI

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:9
  • Category: architecture
  • Problem: inferenceSetAttemptCount() parses NEMOCLAW_SWITCH_SET_ATTEMPTS env var with validation, but this configurability may be YAGNI if not used in CI. Flagged in prior review (PRA-17) and unaddressed.
  • Impact: Unnecessary configuration complexity; no evidence the env var is set in CI.
  • Suggested action: Consider inlining the constant 3 at call sites or removing env configurability if not used in CI.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check if NEMOCLAW_SWITCH_SET_ATTEMPTS is set in any CI workflow or test configuration.
  • Missing regression test: None needed — simplification if YAGNI confirmed.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Function at line 9-15 parses env var with fallback=3; no CI workflow sets NEMOCLAW_SWITCH_SET_ATTEMPTS

PRA-21 Improvement — hostedInstallModel() single-use helper adds indirection

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:50
  • Category: architecture
  • Problem: hostedInstallModel() single-use helper adds indirection; fallback logic could be inlined in env() or made a local constant. Flagged in prior review (PRA-18) and unaddressed.
  • Impact: Unnecessary function call and abstraction layer for one call site.
  • Suggested action: Inline the fallback logic directly in env() or make it a local constant.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:50 — hostedInstallModel() called only from env() at line 62-63.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: hostedInstallModel() at line 50-54 called once at lines 62-63

PRA-22 Improvement — Retry fixture tests missing backoff progression and edge case coverage

  • Location: test/e2e-scenario/support-tests/inference-switch-retry.test.ts:1
  • Category: correctness
  • Problem: Retry fixture tests missing coverage for: delay backoff progression (attempt*5000), boundary attempts (attempts=1), mixed transient/non-transient failure sequences. Flagged in prior review (PRA-20) — partially addressed with new test file but gaps remain.
  • Impact: Critical retry behavior not verified: exponential backoff, single-attempt mode, mixed failure scenarios.
  • Suggested action: Add tests for: delay called with attempt*5000 progression, attempts=1 edge case, transient then non-transient failure sequence.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read test/e2e-scenario/support-tests/inference-switch-retry.test.ts — current tests cover basic retry and no-verify fallback but not backoff progression or attempts=1.
  • Missing regression test: Tests for delay backoff progression, attempts=1, mixed failure sequences.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Current tests: 'retries transient verification failures and preserves verification', 'uses no-verify only after the transient verification budget is exhausted', 'does not bypass non-transient verification failures', 'validates the configured attempt count'

PRA-23 Improvement — workflow-boundary.mts imports inference-switch validator creating coupling

  • Location: tools/e2e-scenarios/workflow-boundary.mts:9
  • Category: architecture
  • Problem: workflow-boundary.mts imports validateInferenceSwitchWorkflowBoundary from inference-switch-workflow-boundary.mts, creating coupling. The new module duplicates validation patterns instead of sharing helpers. Flagged in prior review (PRA-22) and unaddressed.
  • Impact: Tight coupling between modules; duplicated validation logic; harder to maintain consistency.
  • Suggested action: Merge inference-switch-workflow-boundary.mts into workflow-boundary.mts to eliminate duplicate dependency and validation logic.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check tools/e2e-scenarios/workflow-boundary.mts:9 and :7474 — imports and calls validateInferenceSwitchWorkflowBoundary.
  • Missing regression test: Single workflow boundary validator module.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 9: import { validateInferenceSwitchWorkflowBoundary } from "./inference-switch-workflow-boundary.mts"; Line 7474: errors.push(...validateInferenceSwitchWorkflowBoundary(workflowPath));

PRA-24 Improvement — No behavioral test for installHermes --fresh flag

  • Location: test/e2e-scenario/support-tests/inference-switch-retry.test.ts:1
  • Category: correctness
  • Problem: No test for installHermes --fresh flag behavior beyond argv verification in command-shape tests. The --fresh flag is a critical workaround but only tested for argv shape, not actual behavior. Flagged in prior review (PRA-23) and partially addressed.
  • Impact: Cannot verify --fresh actually cleans up failed onboarding state; only argv shape is tested.
  • Suggested action: Add integration test that installHermes with --fresh actually discards failed onboarding state, or document why argv verification is sufficient.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts — test 'discards failed onboarding state before an install attempt' only checks argv.
  • Missing regression test: Integration test verifying --fresh actually cleans up failed onboarding state.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Support test at line 90-100 only verifies command.mock.calls[0][1] includes '--fresh'

PRA-25 Improvement — Support test names could be more behavior-specific

  • Location: test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:100
  • Category: correctness
  • Problem: New support tests added for command shape are good but some test names are generic (e.g., 'retries live PONG probes before returning the final result'). Could be more specific about what behavior is verified.
  • Impact: Test names don't clearly map to requirements; harder to understand coverage at a glance.
  • Suggested action: Consider more descriptive test names that map to specific requirements (e.g., 'retries inference.local PONG probe on transient failure with 5s delay').
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts — test names could be more behavior-specific.
  • Missing regression test: None — test naming improvement.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Test names: 'retries live PONG probes before returning the final result', 'keeps the Anthropic direct probe within the frozen E2E token budget'

PRA-28 Improvement — DEFAULT_HOSTED_INFERENCE_MODEL single-use import

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:18
  • Category: correctness
  • Problem: DEFAULT_HOSTED_INFERENCE_MODEL imported from ../fixtures/hosted-inference.ts but only used as fallback constant in hostedInstallModel(). Single-use import — can be inlined as local constant.
  • Impact: Unnecessary cross-module dependency for a single constant value.
  • Suggested action: Replace import with local constant in helpers.ts: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3';
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45 and test/e2e-scenario/live/hermes-inference-switch-helpers.ts:18 — import only used in hostedInstallModel.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Import at line 18, used only in hostedInstallModel() at line 58

PRA-29 Improvement — DEFAULT_HOSTED_INFERENCE_MODEL single-use import in support test

  • Location: test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:10
  • Category: correctness
  • Problem: Same DEFAULT_HOSTED_INFERENCE_MODEL import in support test — also single-use.
  • Impact: Unnecessary cross-module dependency in test file.
  • Suggested action: Update support test to use local constant or inline value.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:10.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Import at line 10, used only in test at line 63

PRA-30 Improvement — DEFAULT_WORKFLOW_PATH unnecessary config knob

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:11
  • Category: correctness
  • Problem: DEFAULT_WORKFLOW_PATH constant and validateInferenceSwitchWorkflowBoundary() default param create unnecessary config knob — only ever called with default path.
  • Impact: Unnecessary abstraction; function only called with default path in workflow-boundary.mts:7474.
  • Suggested action: Inline the path directly in the function or remove default param since it's always called with the same path.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check tools/e2e-scenarios/inference-switch-workflow-boundary.mts:11, :149, :162 — DEFAULT_WORKFLOW_PATH used only as default param.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Line 11: const DEFAULT_WORKFLOW_PATH = join(REPO_ROOT, ".github", "workflows", "e2e-vitest-scenarios.yaml"); Lines 149, 162: workflowPath = DEFAULT_WORKFLOW_PATH

PRA-31 Improvement — Entire inference-switch-workflow-boundary.mts module duplicates validation logic

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1
  • Category: correctness
  • Problem: Entire inference-switch-workflow-boundary.mts module duplicates validation logic from workflow-boundary.mts (permissions, fail-fast, artifact naming, step ordering) — merge to eliminate duplication (PRA-15, PRA-19, PRA-22).
  • Impact: Duplicated validation logic increases maintenance burden and risk of divergence. Circular-like dependency between modules.
  • Suggested action: Merge validateInferenceSwitchWorkflow into workflow-boundary.mts validateE2eVitestScenariosWorkflowBoundary. Share helper functions for common patterns.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare both files for duplicated validation patterns.
  • Missing regression test: Single consolidated validator module.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: workflow-boundary.mts:9 imports and :7474 calls validateInferenceSwitchWorkflowBoundary; both modules validate permissions.contents === 'read', fail-fast === false, artifact naming
Simplification opportunities: 7 possible cuts, net -183 lines possible

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

  • PRA-19 shrink (tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45): import { DEFAULT_HOSTED_INFERENCE_MODEL } from "../fixtures/hosted-inference.ts";
    • Replacement: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3';
    • Net: 0 lines
    • Safety boundary: Constant value must remain identical; no behavior change
  • PRA-20 yagni (test/e2e-scenario/fixtures/inference-switch-retry.ts:9): export function inferenceSetAttemptCount(raw: string | undefined, fallback = 3): number { if (raw === undefined) return fallback; const parsed = Number(raw); if (!Number.isInteger(parsed) || parsed < 1) { throw new Error(`NEMOCLAW_SWITCH_SET_ATTEMPTS must be a positive integer; got ${raw}`); } return parsed; }
    • Replacement: const INFERENCE_SET_ATTEMPTS = 3;
    • Net: -10 lines
    • Safety boundary: Call sites must use INFERENCE_SET_ATTEMPTS constant; default behavior unchanged
  • PRA-21 shrink (test/e2e-scenario/live/hermes-inference-switch-helpers.ts:50): export function hostedInstallModel(runtimeEnv: NodeJS.ProcessEnv = process.env): string { return (runtimeEnv.NEMOCLAW_MODEL ?? runtimeEnv.NEMOCLAW_COMPAT_MODEL ?? DEFAULT_HOSTED_INFERENCE_MODEL); }
    • Replacement: Inline at env() lines 62-63: NEMOCLAW_MODEL: process.env.NEMOCLAW_MODEL ?? process.env.NEMOCLAW_COMPAT_MODEL ?? DEFAULT_HOSTED_INFERENCE_MODEL, NEMOCLAW_COMPAT_MODEL: process.env.NEMOCLAW_MODEL ?? process.env.NEMOCLAW_COMPAT_MODEL ?? DEFAULT_HOSTED_INFERENCE_MODEL,
    • Net: -5 lines
    • Safety boundary: Fallback logic identical; no behavior change
  • PRA-28 shrink (test/e2e-scenario/live/hermes-inference-switch-helpers.ts:18): import { DEFAULT_HOSTED_INFERENCE_MODEL } from "../fixtures/hosted-inference.ts";
    • Replacement: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3';
    • Net: 0 lines
    • Safety boundary: Constant value must remain identical; no behavior change
  • PRA-29 shrink (test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts:10): import { DEFAULT_HOSTED_INFERENCE_MODEL } from "../fixtures/hosted-inference.ts";
    • Replacement: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3';
    • Net: 0 lines
    • Safety boundary: Constant value must remain identical; no behavior change
  • PRA-30 shrink (tools/e2e-scenarios/inference-switch-workflow-boundary.mts:11): const DEFAULT_WORKFLOW_PATH = join(REPO_ROOT, ".github", "workflows", "e2e-vitest-scenarios.yaml"); and default params workflowPath = DEFAULT_WORKFLOW_PATH
    • Replacement: Remove constant; hardcode path in function bodies or remove default param
    • Net: -3 lines
    • Safety boundary: Path must remain identical; no behavior change
  • PRA-31 delete (tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1): Entire tools/e2e-scenarios/inference-switch-workflow-boundary.mts module (165 lines)
    • Replacement: Merge validation logic into workflow-boundary.mts validateE2eVitestScenariosWorkflowBoundary with shared helpers
    • Net: -165 lines
    • Safety boundary: All validation behavior preserved; no workflow changes
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 Runtime validation — Test that install.sh succeeds without --fresh on clean onboarding state (proves root cause fixed for PRA-6). Workflow matrix changes (.github/workflows/e2e-vitest-scenarios.yaml) and validator modules (tools/e2e-scenarios/inference-switch-workflow-boundary.mts, workflow-boundary.mts) need behavioral runtime validation in CI. The PR evidence shows all 4 matrix cells passed in live run 28345419040.
  • PRA-T2 Runtime validation — Test TRANSIENT_INFERENCE_SET_FAILURE regex: 'temporarily unavailable' matches, 'temporar' alone does not. Workflow matrix changes (.github/workflows/e2e-vitest-scenarios.yaml) and validator modules (tools/e2e-scenarios/inference-switch-workflow-boundary.mts, workflow-boundary.mts) need behavioral runtime validation in CI. The PR evidence shows all 4 matrix cells passed in live run 28345419040.
  • PRA-T3 Runtime validation — Test mockAnthropicEndpointUrl rejects invalid hosts (SSRF prevention). Workflow matrix changes (.github/workflows/e2e-vitest-scenarios.yaml) and validator modules (tools/e2e-scenarios/inference-switch-workflow-boundary.mts, workflow-boundary.mts) need behavioral runtime validation in CI. The PR evidence shows all 4 matrix cells passed in live run 28345419040.
  • PRA-T4 Runtime validation — Test agentReplyContainsToken with substring match: 'the answer is PONG' should pass. Workflow matrix changes (.github/workflows/e2e-vitest-scenarios.yaml) and validator modules (tools/e2e-scenarios/inference-switch-workflow-boundary.mts, workflow-boundary.mts) need behavioral runtime validation in CI. The PR evidence shows all 4 matrix cells passed in live run 28345419040.
  • PRA-T5 Runtime validation — Test parseOpenClawAgentText with malformed JSON wrapper — verify fallback behavior. Workflow matrix changes (.github/workflows/e2e-vitest-scenarios.yaml) and validator modules (tools/e2e-scenarios/inference-switch-workflow-boundary.mts, workflow-boundary.mts) need behavioral runtime validation in CI. The PR evidence shows all 4 matrix cells passed in live run 28345419040.
  • PRA-T6 inferenceSetAttemptCount fallback=3 — None — no test for env var being set in CI. test/e2e-scenario/fixtures/inference-switch-retry.ts:9-15
  • PRA-T7 parseHermesModelBlock silent catch — None — no test with malformed JSON. test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142
  • PRA-T8 TRANSIENT_INFERENCE_SET_FAILURE regex workaround — None — no test documenting which provider errors are transient. test/e2e-scenario/fixtures/inference-switch-retry.ts:7
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: inferenceSetAttemptCount fallback=3

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test for env var being set in CI
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:9-15

PRA-2 Resolve/justify — Source-of-truth review needed: parseHermesModelBlock silent catch

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test with malformed JSON
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:142

PRA-3 Resolve/justify — Source-of-truth review needed: TRANSIENT_INFERENCE_SET_FAILURE regex workaround

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None — no test documenting which provider errors are transient
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:7

PRA-4 Resolve/justify — Source-of-truth review needed: agentReplyContainsToken strict equality

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Support test 'openclaw-inference-switch agent reply matching tolerates wrapped PONG' exists but expects strict equality
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts:708

PRA-5 Resolve/justify — Source-of-truth review needed: parseOpenClawAgentText regex JSON fallback

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Support test exists but uses regex fallback behavior
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts:717

PRA-6 Required — installHermes --fresh workaround lacks TODO linking to root-cause fix

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: architecture
  • Problem: installHermes() unconditionally passes --fresh flag to install.sh to discard failed onboarding state, but no TODO comment references a GitHub issue tracking the onboarding cleanup root-cause fix. This was flagged in the prior review (PRA-5) and remains unaddressed.
  • Impact: If onboarding failure mode regresses or changes, --fresh may mask the symptom without fixing the underlying state leak. No visibility into whether the root cause is being tracked.
  • Required action: Add a TODO comment at line 328 referencing a GitHub issue tracking the onboarding cleanup fix, or document why --fresh is the correct permanent solution.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 — installHermes passes ['install.sh', '--non-interactive', '--fresh', '--yes-i-accept-third-party-software']; check for TODO comment referencing follow-up issue.
  • Missing regression test: GitHub issue tracking the root-cause fix with acceptance criteria for clean onboarding state.
  • Done when: The required change is committed and verification passes: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 — installHermes passes ['install.sh', '--non-interactive', '--fresh', '--yes-i-accept-third-party-software']; check for TODO comment referencing follow-up issue.
  • Evidence: installHermes() at helpers.ts:328 passes --fresh; support test at hermes-inference-switch-command-shape.test.ts:90 only verifies argv shape

PRA-7 Required — --fresh flag masks onboarding state leak without regression test

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: correctness
  • Problem: The --fresh flag masks potential onboarding state leak without a regression test proving install.sh succeeds without --fresh when onboarding state is clean. Flagged in prior review (PRA-6) and unaddressed.
  • Impact: Cannot verify the root cause is actually fixed; the workaround may become permanent without detection.
  • Required action: Add a test that install.sh succeeds without --fresh when onboarding state is clean, proving the root cause is fixed. Alternatively, fix onboarding cleanup at source in install.sh.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check if any test verifies install.sh succeeds without --fresh on a clean sandbox state.
  • Missing regression test: Test that install.sh --non-interactive (without --fresh) succeeds on a pristine sandbox, demonstrating the root cause is fixed.
  • Done when: The required change is committed and verification passes: Check if any test verifies install.sh succeeds without --fresh on a clean sandbox state.
  • Evidence: No existing test covers install.sh without --fresh on clean state

PRA-8 Required — --fresh workaround lacks source-of-truth justification comment

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328
  • Category: correctness
  • Problem: The --fresh workaround lacks a source-of-truth justification comment explaining: the invalid state (failed onboarding leaves sandbox unusable), source boundary (install.sh onboarding logic), source-fix constraint (requires install.sh change outside this PR), regression test proving source fixed, and removal condition. Flagged in prior review (PRA-7) and unaddressed.
  • Impact: Without documentation, the workaround cannot be evaluated for removal when the root cause is fixed. Maintainers may not know it exists or why.
  • Required action: Add a code comment at line 328 with: the invalid state, source boundary, source-fix constraint, regression test proving source fixed, and removal condition.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 and surrounding lines for source-of-truth justification comment.
  • Missing regression test: Source-of-truth comment documenting all five elements at the workaround site.
  • Done when: The required change is committed and verification passes: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:328 and surrounding lines for source-of-truth justification comment.
  • Evidence: No comment at installHermes() call site explaining the workaround rationale

PRA-9 Resolve/justify — TRANSIENT_INFERENCE_SET_FAILURE regex over-matches with bare 'temporar' prefix

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:7
  • Category: architecture
  • Problem: TRANSIENT_INFERENCE_SET_FAILURE regex includes bare 'temporar' prefix which over-matches (e.g., 'temporary', 'temporarily' but also could match unrelated words containing 'temporar'). Flagged in prior review (PRA-10/PRA-14) and unaddressed.
  • Impact: May incorrectly classify non-transient errors as transient, causing inappropriate retries and --no-verify fallback.
  • Recommended action: Tighten regex to provider-specific patterns: 'temporarily unavailable', 'temporary failure', 'temporary error'. Remove bare 'temporar' prefix.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex contains '|temporar' without word boundary or provider context.
  • Missing regression test: Test that 'temporarily unavailable' matches but 'temporar' alone does not.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex contains '|temporar' without word boundary or provider context.
  • Evidence: Regex: /timed? out|timeout|ETIMEDOUT|ECONNRESET|EAI_AGAIN|ENOTFOUND|failed to connect|error sending request|failed to verify inference endpoint|502|503|504|temporar/iu

PRA-10 Resolve/justify — mockAnthropicEndpointUrl accepts arbitrary host without allowlist (SSRF risk)

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40
  • Category: security
  • Problem: mockAnthropicEndpointUrl() accepts arbitrary host via NEMOCLAW_SWITCH_MOCK_HOST env var without allowlist validation. In a test context this is lower risk, but could enable SSRF if env var is attacker-controlled. Flagged in prior review (PRA-13) and unaddressed.
  • Impact: If NEMOCLAW_SWITCH_MOCK_HOST is set to an internal or external address, the mock server could be reached at that address, potentially enabling SSRF.
  • Recommended action: Add allowlist validation (e.g., only '127.0.0.1', 'host.openshell.internal', 'localhost') or document as test-only trusted input with a comment explaining the trust boundary.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40 — function uses runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal' without validation.
  • Missing regression test: Test that invalid host values are rejected or sanitized.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:40 — function uses runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal' without validation.
  • Evidence: mockAnthropicEndpointUrl(port, runtimeEnv) returns `http://${runtimeEnv.NEMOCLAW_SWITCH_MOCK_HOST ?? 'host.openshell.internal'}:${port}`

PRA-11 Resolve/justify — env() sets both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL without comment

  • Location: test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62
  • Category: architecture
  • Problem: env() sets both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL to the same value (hostedInstallModel()) without a comment explaining why both are needed. Flagged in prior review (PRA-12) and unaddressed.
  • Impact: Future maintainers may not understand the dual assignment and could break the hosted-inference.ts contract.
  • Recommended action: Add a brief comment explaining why both are set (matches hosted-inference.ts contract), or consider a helper to set both atomically.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62-63 — both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL assigned hostedInstallModel() with no explanatory comment.
  • Missing regression test: Documentation comment at the assignment site.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/hermes-inference-switch-helpers.ts:62-63 — both NEMOCLAW_MODEL and NEMOCLAW_COMPAT_MODEL assigned hostedInstallModel() with no explanatory comment.
  • Evidence: Lines 62-63: NEMOCLAW_MODEL: hostedInstallModel(), NEMOCLAW_COMPAT_MODEL: hostedInstallModel()

PRA-12 Resolve/justify — isDeepStrictEqual matrix ratchet lacks documentation

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84
  • Category: architecture
  • Problem: validateJob uses isDeepStrictEqual to compare the entire matrix.include array against expectedModes(). This creates a ratchet: adding new modes requires updating both the expectedModes() function and the validation logic, with no comment explaining this coupling. Flagged in prior review (PRA-11) and unaddressed.
  • Impact: Adding a new mode (e.g., a third provider) will fail validation silently unless both places are updated. No guidance for maintainers.
  • Recommended action: Add a code comment at expectedModes() explaining that adding modes requires updating this function and the validation, or consider a 'must contain at least these two modes' approach using subset matching.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84 — isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)) with no comment about the ratchet.
  • Missing regression test: Comment documenting the ratchet, or refactor to subset validation.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read tools/e2e-scenarios/inference-switch-workflow-boundary.mts:84 — isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)) with no comment about the ratchet.
  • Evidence: Line 84: if (!isDeepStrictEqual(job.strategy?.matrix?.include, expectedModes(spec.agent)))

PRA-13 Resolve/justify — New validator module duplicates workflow-boundary.mts validation patterns

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:1
  • Category: architecture
  • Problem: New validator module inference-switch-workflow-boundary.mts duplicates validation patterns from workflow-boundary.mts (permission checks, step ordering, artifact naming) rather than merging into the existing validateE2eVitestScenariosWorkflowBoundary. Creates cross-module coupling (workflow-boundary.mts imports it). Flagged in prior review (PRA-15, PRA-19) and unaddressed.
  • Impact: Duplicated validation logic increases maintenance burden and risk of divergence. Circular-like dependency between modules.
  • Recommended action: Merge validateInferenceSwitchWorkflow into workflow-boundary.mts validateE2eVitestScenariosWorkflowBoundary. Keep validation DRY and avoid circular-like dependency.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare tools/e2e-scenarios/inference-switch-workflow-boundary.mts and workflow-boundary.mts for duplicated patterns (permissions, fail-fast, artifact naming, step ordering).
  • Missing regression test: Single consolidated validator with shared helper functions.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare tools/e2e-scenarios/inference-switch-workflow-boundary.mts and workflow-boundary.mts for duplicated patterns (permissions, fail-fast, artifact naming, step ordering).
  • Evidence: workflow-boundary.mts:9 imports and :7474 calls validateInferenceSwitchWorkflowBoundary; both modules validate permissions.contents === 'read', fail-fast === false, artifact naming

PRA-14 Resolve/justify — agentReplyContainsToken uses strict equality causing false negatives

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:708
  • Category: architecture
  • Problem: agentReplyContainsToken uses strict equality (normalizedReply === normalizedExpected) causing false negatives when agent output contains extra text around 'PONG'. Flagged in prior review (PRA-1/PRA-8) and unaddressed.
  • Impact: Valid agent responses containing 'PONG' with additional context (e.g., 'The answer is PONG') will fail the test, reducing test reliability.
  • Recommended action: Change to substring/includes check (normalizedReply.includes(normalizedExpected)) or document why exact match is required for this test's contract.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:708 — function returns normalizedReply === normalizedExpected.
  • Missing regression test: Test that 'the answer is PONG' passes (currently fails with strict equality).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:708 — function returns normalizedReply === normalizedExpected.
  • Evidence: Line 708: return normalizedReply === normalizedExpected;

PRA-15 Resolve/justify — parseOpenClawAgentText uses fragile regex-based JSON fallback

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:717
  • Category: architecture
  • Problem: parseOpenClawAgentText uses fragile regex-based JSON fallback (/{/g) to find JSON envelopes in agent output. This is a localized workaround for malformed agent output. Flagged in prior review (PRA-2/PRA-9) and unaddressed.
  • Impact: Fragile parsing may break with format changes, miss valid JSON, or accept invalid JSON. No clear contract for agent output format.
  • Recommended action: Replace with a proper streaming JSON parser or require the agent output to be strict JSON. If fallback is necessary, add a comment explaining why the source cannot produce clean JSON and what regression test proves it cannot regress.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:717 — regex /{/g used to find JSON start positions.
  • Missing regression test: Source-of-truth comment documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/live/openclaw-inference-switch.test.ts:717 — regex /{/g used to find JSON start positions.
  • Evidence: Line 717: const decoder = new RegExp('{', 'g'); while ((match = decoder.exec(raw)) !== null)

PRA-16 Resolve/justify — TRANSIENT_INFERENCE_SET_FAILURE regex lacks source-of-truth justification

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:7
  • Category: architecture
  • Problem: TRANSIENT_INFERENCE_SET_FAILURE regex is a localized workaround for transient provider failures without source-of-truth justification. Flagged in prior review (PRA-3) and unaddressed.
  • Impact: Cannot evaluate when this workaround can be removed or if the provider API has improved. No visibility into what specific transient errors are expected.
  • Recommended action: Add a comment at the regex definition explaining: what invalid state is handled (transient network/provider errors), where that state is created (provider API), why the source cannot be fixed in this PR (external API), what regression test proves the source cannot regress, and when the workaround can be removed.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex definition with no source-of-truth comment.
  • Missing regression test: Source-of-truth comment at regex definition site.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts:7 — regex definition with no source-of-truth comment.
  • Evidence: Line 7: const TRANSIENT_INFERENCE_SET_FAILURE = /timed? out|timeout|...|temporar/iu;

PRA-17 Resolve/justify — NVIDIA_INFERENCE_API_KEY secret used for mock Anthropic mode

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:906
  • Category: security
  • Problem: The hermes-inference-switch-vitest and openclaw-inference-switch-vitest jobs use matrix strategy with mode-specific sandbox names and artifact paths, but the workflow still uses NVIDIA_INFERENCE_API_KEY secret for both modes. The anthropic mode uses a mock provider but still requires the NVIDIA secret for onboarding.
  • Impact: Over-privileged secret usage; mock mode doesn't need real NVIDIA credentials. Could expose production credentials in test logs if misconfigured.
  • Recommended action: Verify that NVIDIA_INFERENCE_API_KEY secret is appropriately scoped and not over-privileged for the mock anthropic mode. Consider if a separate mock credential would be cleaner.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:906-940 — both matrix modes use NVIDIA_INFERENCE_API_KEY secret.
  • Missing regression test: Secret scope documentation in workflow comments.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:906-940 — both matrix modes use NVIDIA_INFERENCE_API_KEY secret.
  • Evidence: Both matrix modes in hermes-inference-switch-vitest and openclaw-inference-switch-vitest jobs have env: NVIDIA_INFERENCE_API_KEY: ${{ secrets.NVIDIA_INFERENCE_API_KEY }}

PRA-18 Resolve/justify — OpenClaw job step reordering may break Docker login

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:4700
  • Category: correctness
  • Problem: openclaw-inference-switch-vitest job reorders steps: 'Configure isolated Docker auth directory' and 'Authenticate to Docker Hub' now run before 'Set up Node'/'Install root dependencies'/'Build CLI'. This changes the execution order — Docker auth config now runs before Node setup. Need to verify this doesn't break Docker login (which needs docker CLI).
  • Impact: If docker CLI is not available at the Docker auth step, the login will fail, breaking the OpenClaw test.
  • Recommended action: Verify that docker CLI is available before 'Configure isolated Docker auth directory' step runs. The runner should have docker pre-installed, but explicit check would be safer.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:4700-4760 — step order changed; docker login step runs after 'Authenticate to Docker Hub' but before Node setup.
  • Missing regression test: CI verification that docker CLI is available at Docker auth step.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check .github/workflows/e2e-vitest-scenarios.yaml:4700-4760 — step order changed; docker login step runs after 'Authenticate to Docker Hub' but before Node setup.
  • Evidence: Steps 4700-4760: Configure isolated Docker auth directory -> Authenticate to Docker Hub -> Set up Node -> Install root dependencies -> Build CLI

PRA-19 Improvement — DEFAULT_HOSTED_INFERENCE_MODEL import only used as fallback constant

  • Location: tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45
  • Category: architecture
  • Problem: DEFAULT_HOSTED_INFERENCE_MODEL imported from ../fixtures/hosted-inference.ts but only used as fallback constant in hostedInstallModel(). Could be inlined as local constant to reduce cross-module dependency. Flagged in prior review (PRA-16) and unaddressed.
  • Impact: Unnecessary cross-module dependency for a single constant value.
  • Suggested action: Replace import with local constant: const DEFAULT_HOSTED_INFERENCE_MODEL = 'nvidia/nvidia/nemotron-3-super-v3'; in helpers.ts. Also update support test import.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check tools/e2e-scenarios/inference-switch-workflow-boundary.mts:45 — import used only in hostedInstallModel fallback.
  • Missing regression test: None needed — simplification.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Import at line 45, used only in hostedInstallModel() at line 58

PRA-20 Improvement — inferenceSetAttemptCount env configurability may be YAGNI

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:9
  • Category: architecture
  • Problem: inferenceSetAttemptCount() parses NEMOCLAW_SWITCH_SET_ATTEMPTS env var with validation, but this configurability may be YAGNI if not used in CI. Flagged in prior review (PRA-17) and unaddressed.
  • Impact: Unnecessary configuration complexity; no evidence the env var is set in CI.
  • Suggested action: Consider inlining the constant 3 at call sites or removing env configurability if not used in CI.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check if NEMOCLAW_SWITCH_SET_ATTEMPTS is set in any CI workflow or test configuration.
  • Missing regression test: None needed — simplification if YAGNI confirmed.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Function at line 9-15 parses env var with fallback=3; no CI workflow sets NEMOCLAW_SWITCH_SET_ATTEMPTS

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

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: runInferenceSetWithRetry() transient --no-verify fallback.
Open items: 0 required · 4 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 4 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: runInferenceSetWithRetry() transient --no-verify fallback
  • PRA-2 Resolve or justify: Source-of-truth review needed: OpenClaw post-switch inference.local and agent skip paths
  • PRA-3 Resolve or justify: Expose or remove the silent inference-set --no-verify fallback in test/e2e-scenario/fixtures/inference-switch-retry.ts:36
  • PRA-4 Resolve or justify: Do not skip required OpenClaw runtime evidence after a possible unverified switch in test/e2e-scenario/live/openclaw-inference-switch.test.ts:988
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: runInferenceSetWithRetry() transient --no-verify fallback
  • PRA-T8 Add or justify test follow-up: OpenClaw post-switch inference.local and agent skip paths

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify security test/e2e-scenario/fixtures/inference-switch-retry.ts:36 Either remove the --no-verify fallback, or make it explicit in the helper contract. Return a typed value such as { result, verificationUsed, usedNoVerifyFallback }, propagate it through runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(), and write the final verification mode into scenario artifacts. If the fallback remains, document the invalid state, source boundary, source-fix constraint, and removal condition next to the helper.
PRA-4 Resolve/justify correctness test/e2e-scenario/live/openclaw-inference-switch.test.ts:988 After the retry helper exposes final verification mode, fail rather than skip when inference.local or the OpenClaw agent proof is absent after a no-verify switch. Keep transient skips only for fully verified switches, or add an explicit accepted-risk gate plus an artifact field that makes the reduced evidence impossible to miss.
Review findings by urgency: 0 required fixes, 4 items to resolve/justify, 0 in-scope improvements

⚠️ 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.

PRA-1 Resolve/justify — Source-of-truth review needed: runInferenceSetWithRetry() transient --no-verify fallback

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Existing tests prove retry mechanics, fallback after budget exhaustion, and no bypass for non-transient failures, but no test proves final verification mode is returned to callers or written to artifacts.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts returns options.run(attempt, false) after the final transient failure and still returns Promise<ShellProbeResult>.

PRA-2 Resolve/justify — Source-of-truth review needed: OpenClaw post-switch inference.local and agent skip paths

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: No changed support test proves inference.local and agent evidence are mandatory when the inference-set step used --no-verify.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts writes skipped scenario-result.json and calls skip() for inference.local failures and agent timeouts after accepting switchResult.exitCode.

PRA-3 Resolve/justify — Expose or remove the silent inference-set --no-verify fallback

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:36
  • Category: security
  • Problem: runInferenceSetWithRetry() retries verified inference-set attempts, but after the last transient-looking verification failure it calls options.run(attempt, false). The Hermes and OpenClaw callers translate verify=false into --no-verify while the helper still returns only ShellProbeResult, so callers and scenario-result artifacts cannot distinguish a verified switch from a switch that bypassed endpoint verification.
  • Impact: Endpoint verification is part of the inference routing trust boundary. A provider-route, SSRF/policy, hosted-inference, or route-validation regression that emits transient-looking output can be normalized into a successful switch, and reviewers only see a successful result without knowing verification was bypassed.
  • Recommended action: Either remove the --no-verify fallback, or make it explicit in the helper contract. Return a typed value such as { result, verificationUsed, usedNoVerifyFallback }, propagate it through runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(), and write the final verification mode into scenario artifacts. If the fallback remains, document the invalid state, source boundary, source-fix constraint, and removal condition next to the helper.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts around the final options.run(attempt, false) branch, then trace runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(); confirm whether the returned value and scenario-result artifacts expose that --no-verify was used.
  • Missing regression test: Add a focused support test where verified attempts exhaust the transient budget and the no-verify fallback succeeds; assert the helper result exposes verificationUsed: false or usedNoVerifyFallback: true. Add the verified-success counterpart asserting verificationUsed: true, and cover Hermes/OpenClaw artifact writing of that field.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts around the final options.run(attempt, false) branch, then trace runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(); confirm whether the returned value and scenario-result artifacts expose that --no-verify was used.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:31-37 calls options.run(attempt, true) for verified attempts, then returns options.run(attempt, false) after the last transient failure. test/e2e-scenario/live/hermes-inference-switch-helpers.ts and test/e2e-scenario/live/openclaw-inference-switch.test.ts append --no-verify when verify is false, but both paths return ShellProbeResult only.

PRA-4 Resolve/justify — Do not skip required OpenClaw runtime evidence after a possible unverified switch

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:988
  • Category: correctness
  • Problem: The OpenClaw scenario accepts switchResult.exitCode, then checks route/config/registry state, but still converts inference.local failures or OpenClaw agent timeouts into skipped scenario results. Because switchResult carries no final verification-mode signal, those skips can happen even when the switch succeeded only through the --no-verify fallback.
  • Impact: The lane can report an accepted skip after route/config checks even when endpoint verification was bypassed and the independent runtime proof is missing. That weakens detection of broken inference routing, provider integration, sandbox-boundary, or policy regressions.
  • Recommended action: After the retry helper exposes final verification mode, fail rather than skip when inference.local or the OpenClaw agent proof is absent after a no-verify switch. Keep transient skips only for fully verified switches, or add an explicit accepted-risk gate plus an artifact field that makes the reduced evidence impossible to miss.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect test/e2e-scenario/live/openclaw-inference-switch.test.ts around the scenario-result.json writes for inference.skipped and agentTurn.skipped, then trace whether switchResult includes any usedNoVerifyFallback or verificationUsed signal.
  • Missing regression test: Add a support seam where inference-set fallback succeeds with --no-verify and checkSandboxInference() returns a transient skipped result; assert the scenario fails instead of writing status: skipped. Add the analogous agent-timeout case, or a shared helper test covering both post-switch evidence paths.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect test/e2e-scenario/live/openclaw-inference-switch.test.ts around the scenario-result.json writes for inference.skipped and agentTurn.skipped, then trace whether switchResult includes any usedNoVerifyFallback or verificationUsed signal.
  • Evidence: After expect(switchResult.exitCode).toBe(0), the test writes status: skipped and calls skip() for both inference.local and agent-turn failures, while switchResult is a plain ShellProbeResult with no verification-mode field.

💡 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.

  • None.
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 Runtime validation — runInferenceSetWithRetry exposes usedNoVerifyFallback when the final unverified attempt succeeds. The changed surfaces include workflow trusted-code boundaries, Docker credential handling, live sandbox routing, provider endpoint verification, and inference.local/agent runtime behavior. Static support tests cover much of the scaffolding, but the risky fallback and post-switch evidence paths need behavior-specific contract coverage.
  • PRA-T2 Runtime validation — runInferenceSetWithRetry exposes verificationUsed true when a verified retry succeeds. The changed surfaces include workflow trusted-code boundaries, Docker credential handling, live sandbox routing, provider endpoint verification, and inference.local/agent runtime behavior. Static support tests cover much of the scaffolding, but the risky fallback and post-switch evidence paths need behavior-specific contract coverage.
  • PRA-T3 Runtime validation — Hermes and OpenClaw scenario-result artifacts include the final inference-set verification mode. The changed surfaces include workflow trusted-code boundaries, Docker credential handling, live sandbox routing, provider endpoint verification, and inference.local/agent runtime behavior. Static support tests cover much of the scaffolding, but the risky fallback and post-switch evidence paths need behavior-specific contract coverage.
  • PRA-T4 Runtime validation — OpenClaw inference.local skip fails or records an explicit accepted-risk artifact when the switch used --no-verify. The changed surfaces include workflow trusted-code boundaries, Docker credential handling, live sandbox routing, provider endpoint verification, and inference.local/agent runtime behavior. Static support tests cover much of the scaffolding, but the risky fallback and post-switch evidence paths need behavior-specific contract coverage.
  • PRA-T5 Runtime validation — OpenClaw agent timeout skip fails or records an explicit accepted-risk artifact when the switch used --no-verify. The changed surfaces include workflow trusted-code boundaries, Docker credential handling, live sandbox routing, provider endpoint verification, and inference.local/agent runtime behavior. Static support tests cover much of the scaffolding, but the risky fallback and post-switch evidence paths need behavior-specific contract coverage.
  • PRA-T6 Acceptance clause — No deterministic linked issue acceptance clauses were available in the supplied context (linkedIssues: []). — add test evidence or identify existing coverage. The PR body mentions an issue reference, but PR-provided text is untrusted and no linked issue/comment payload was provided for literal clause extraction. Review therefore maps only the changed-code behavior and does not claim issue acceptance coverage.
  • PRA-T7 runInferenceSetWithRetry() transient --no-verify fallback — Existing tests prove retry mechanics, fallback after budget exhaustion, and no bypass for non-transient failures, but no test proves final verification mode is returned to callers or written to artifacts.. test/e2e-scenario/fixtures/inference-switch-retry.ts returns options.run(attempt, false) after the final transient failure and still returns Promise<ShellProbeResult>.
  • PRA-T8 OpenClaw post-switch inference.local and agent skip paths — No changed support test proves inference.local and agent evidence are mandatory when the inference-set step used --no-verify.. test/e2e-scenario/live/openclaw-inference-switch.test.ts writes skipped scenario-result.json and calls skip() for inference.local failures and agent timeouts after accepting switchResult.exitCode.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: runInferenceSetWithRetry() transient --no-verify fallback

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Existing tests prove retry mechanics, fallback after budget exhaustion, and no bypass for non-transient failures, but no test proves final verification mode is returned to callers or written to artifacts.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts returns options.run(attempt, false) after the final transient failure and still returns Promise<ShellProbeResult>.

PRA-2 Resolve/justify — Source-of-truth review needed: OpenClaw post-switch inference.local and agent skip paths

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: No changed support test proves inference.local and agent evidence are mandatory when the inference-set step used --no-verify.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/e2e-scenario/live/openclaw-inference-switch.test.ts writes skipped scenario-result.json and calls skip() for inference.local failures and agent timeouts after accepting switchResult.exitCode.

PRA-3 Resolve/justify — Expose or remove the silent inference-set --no-verify fallback

  • Location: test/e2e-scenario/fixtures/inference-switch-retry.ts:36
  • Category: security
  • Problem: runInferenceSetWithRetry() retries verified inference-set attempts, but after the last transient-looking verification failure it calls options.run(attempt, false). The Hermes and OpenClaw callers translate verify=false into --no-verify while the helper still returns only ShellProbeResult, so callers and scenario-result artifacts cannot distinguish a verified switch from a switch that bypassed endpoint verification.
  • Impact: Endpoint verification is part of the inference routing trust boundary. A provider-route, SSRF/policy, hosted-inference, or route-validation regression that emits transient-looking output can be normalized into a successful switch, and reviewers only see a successful result without knowing verification was bypassed.
  • Recommended action: Either remove the --no-verify fallback, or make it explicit in the helper contract. Return a typed value such as { result, verificationUsed, usedNoVerifyFallback }, propagate it through runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(), and write the final verification mode into scenario artifacts. If the fallback remains, document the invalid state, source boundary, source-fix constraint, and removal condition next to the helper.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts around the final options.run(attempt, false) branch, then trace runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(); confirm whether the returned value and scenario-result artifacts expose that --no-verify was used.
  • Missing regression test: Add a focused support test where verified attempts exhaust the transient budget and the no-verify fallback succeeds; assert the helper result exposes verificationUsed: false or usedNoVerifyFallback: true. Add the verified-success counterpart asserting verificationUsed: true, and cover Hermes/OpenClaw artifact writing of that field.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read test/e2e-scenario/fixtures/inference-switch-retry.ts around the final options.run(attempt, false) branch, then trace runHermesInferenceSetWithRetry() and runOpenClawInferenceSetWithRetry(); confirm whether the returned value and scenario-result artifacts expose that --no-verify was used.
  • Evidence: test/e2e-scenario/fixtures/inference-switch-retry.ts:31-37 calls options.run(attempt, true) for verified attempts, then returns options.run(attempt, false) after the last transient failure. test/e2e-scenario/live/hermes-inference-switch-helpers.ts and test/e2e-scenario/live/openclaw-inference-switch.test.ts append --no-verify when verify is false, but both paths return ShellProbeResult only.

PRA-4 Resolve/justify — Do not skip required OpenClaw runtime evidence after a possible unverified switch

  • Location: test/e2e-scenario/live/openclaw-inference-switch.test.ts:988
  • Category: correctness
  • Problem: The OpenClaw scenario accepts switchResult.exitCode, then checks route/config/registry state, but still converts inference.local failures or OpenClaw agent timeouts into skipped scenario results. Because switchResult carries no final verification-mode signal, those skips can happen even when the switch succeeded only through the --no-verify fallback.
  • Impact: The lane can report an accepted skip after route/config checks even when endpoint verification was bypassed and the independent runtime proof is missing. That weakens detection of broken inference routing, provider integration, sandbox-boundary, or policy regressions.
  • Recommended action: After the retry helper exposes final verification mode, fail rather than skip when inference.local or the OpenClaw agent proof is absent after a no-verify switch. Keep transient skips only for fully verified switches, or add an explicit accepted-risk gate plus an artifact field that makes the reduced evidence impossible to miss.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect test/e2e-scenario/live/openclaw-inference-switch.test.ts around the scenario-result.json writes for inference.skipped and agentTurn.skipped, then trace whether switchResult includes any usedNoVerifyFallback or verificationUsed signal.
  • Missing regression test: Add a support seam where inference-set fallback succeeds with --no-verify and checkSandboxInference() returns a transient skipped result; assert the scenario fails instead of writing status: skipped. Add the analogous agent-timeout case, or a shared helper test covering both post-switch evidence paths.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect test/e2e-scenario/live/openclaw-inference-switch.test.ts around the scenario-result.json writes for inference.skipped and agentTurn.skipped, then trace whether switchResult includes any usedNoVerifyFallback or verificationUsed signal.
  • Evidence: After expect(switchResult.exitCode).toBe(0), the test writes status: skipped and calls skip() for both inference.local and agent-turn failures, while switchResult is a plain ShellProbeResult with no verification-mode field.

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

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28338428225
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ⚠️ Run cancelled — no signal

Run: 28338428908
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: openclaw-inference-switch-vitest
Summary: 0 passed, 0 failed, 1 cancelled, 0 skipped

Job Result
openclaw-inference-switch-vitest ⚠️ cancelled

cv added 2 commits June 28, 2026 15:44
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28338605942
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: openclaw-inference-switch-vitest
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
openclaw-inference-switch-vitest ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28338605200
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest
Summary: 0 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance provider: anthropic Anthropic or Claude provider behavior labels Jun 29, 2026
cv added a commit that referenced this pull request Jun 29, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Advance the default Hermes final-image base to the newly published
post-stale-layout digest and retire the temporary digest-coupled repair
that intentionally rejected newer published bases. This restores Hermes
onboarding and live E2E builds after the `latest` base moved, while
keeping current-state fail-closed layout checks and the older
`.hermes-data` compatibility migration.

## Related Issue

Unblocks #5947. Follow-up to #5882.

## Changes

- Pin the default Hermes base to
`sha256:8dad3b989a9ed1e601743310b97be21be5f59f89f7913a47d04f3ec3c40b8ce6`,
whose published ARM64 image has neither `/sandbox/.openclaw` nor
`/sandbox/.hermes-data`.
- Remove the fired `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST` guard and
temporary `/sandbox/.openclaw` repair; replace it with a small
fail-closed invariant that rejects retired OpenClaw state rather than
maintaining it.
- Keep published-image provenance in the resolver: export immutable
official GHCR digests and reject candidates containing `.openclaw` or
`.hermes-data` state, while preserving local rebuild and caller-selected
base refs in the final Dockerfile.
- Delete the dedicated stale-layout verifier job, script, helpers, and
tests now that their documented removal trigger has fired.
- Add a focused final-image layout suite covering OpenClaw-state
refusal, the retained `.hermes-data` migration, and symlink refusal;
keep immutable published-ref coverage at the resolver boundary and
extend live secret-boundary inspection to both retired paths.
- Reject nested symlinks before copying retained `.hermes-data`
compatibility state so migration cannot preserve links outside the
legacy tree.

## Type of Change

- [x] 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
<!-- 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:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: internal published-image
lifecycle cleanup; CLI behavior, configuration, supported workflows, and
the documented Hermes state location are unchanged.
- [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: pending independent
review of the Hermes sandbox image boundary.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## 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
- [ ] 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)
- [ ] 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)

Verification evidence:

- Pulled and inspected the published ARM64 `8dad…` image;
`/sandbox/.openclaw` and `/sandbox/.hermes-data` are absent.
- Built `agents/hermes/Dockerfile` against the exact `8dad…` digest and
verified sandbox-user readability, runtime permissions, runtime
symlinks, and absence of both stale paths.
- Focused integration run passed 61 tests across
`hermes-final-image-layout`, `hermes-doctor-config-hash`,
`pr-workflow-contract`, and `sandbox-provisioning`.
- `npm run build:cli`, `npm run typecheck:cli`, `npm run checks`, and
`npm run test:projects:check` passed.
- Normal commit and push hooks passed for all commits, including the
full CLI test lane, shellcheck, hadolint, repository checks,
source-shape and test-size budgets, gitleaks, commitlint, and CLI
TypeScript checks.
- [Vitest E2E run
28339846140](https://github.com/NVIDIA/NemoClaw/actions/runs/28339846140)
passed Hermes install/inference, root entrypoint, and secret-boundary
jobs, then exposed that Dockerfile-level registry validation rejected
the forced local `:latest` base-cache rebuild. Commit `ce85e762a` moves
provenance back to the resolver boundary; [exact-head rerun
28340627350](https://github.com/NVIDIA/NemoClaw/actions/runs/28340627350)
passed all four jobs, including stale-base rebuild.
- Downloaded exact-head artifacts have no credential-shaped matches;
every command result in the rebuild artifact is zero, post-rebuild
inference returned `PONG`, backup scanning found no leaks, and all
scenario cleanups report no failures.
- Required documentation writer assessment, rerun after the fail-closed
review fixes, reported no user-facing docs changes needed.

---
<!-- 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>
@cv cv added the v0.0.70 label Jun 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28344174367
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 1 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure
openclaw-inference-switch-vitest ✅ success

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28344757054
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 1 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure
openclaw-inference-switch-vitest ✅ success

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28345419040
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ✅ success
openclaw-inference-switch-vitest ✅ success

@cv

cv commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Advisor disposition at final head aa4591030f128b4904205e004e802395913af4b6:

  • PRA-1/4/5/6 (--fresh): not a product workaround. This E2E owns a deterministic sandbox and sets NEMOCLAW_RECREATE_SANDBOX=1; every attempt must start a fresh acceptance-test fixture rather than resume state left by an earlier failed attempt. Resume and clean-state-without---fresh behavior are already separate onboard-resume product contracts. A TODO to remove isolation would be misleading.
  • GPT PRA-1/2 and retry follow-ups: resolved by source and runtime evidence. The shared helper first makes three verified attempts, recognizes only the frozen v0.0.69 transient classes, and never bypasses a non-transient failure. Only then does it use --no-verify; success still requires the independent OpenShell route, agent config/hash, registry/session, inference.local, and agent/API chat assertions. Run 28345419040 passed all four cells. Hermes Anthropic artifacts show the three verified failures, a zero-exit fallback, and zero-exit downstream route/config/hash/inference/API proof.
  • PRA-3/14 (temporar): intentional compatibility with the frozen shared shell helper, which used the same prefix. Even an overmatched message cannot make this E2E pass because the live boundaries above remain mandatory. Narrowing it here would silently weaken parity with the old job.
  • PRA-7 exact matrix match: intentional current-state ratchet. A new mode must consciously update its sandbox name, provider/API/model values, artifacts, credential boundary, and mutation tests; silently accepting an extra matrix row would defeat this PR's purpose.
  • PRA-8/10/16: NEMOCLAW_MODEL is the installer-facing selection and NEMOCLAW_COMPAT_MODEL is the hosted fixture/default contract. They deliberately carry the same initial hosted model so the Anthropic switch target cannot leak backward into onboarding. The focused test and both live Hermes modes prove that separation. DEFAULT_HOSTED_INFERENCE_MODEL is the required final fallback.
  • PRA-9 validator module: deliberate separation of concerns. The central workflow validator is already about 7,500 lines; the small inference-switch validator owns one cohesive mode/security contract and is composed once by the central gate. The integration mutation test proves the composition.
  • PRA-2/11/12 OpenClaw JSON/PONG parsing: pre-existing code outside this PR's changed hunks. The final run nevertheless proved exact PONG at the real agent boundary in both modes.
  • PRA-13: broad workflow permissions are pre-existing and outside this slice. Both changed jobs pin contents: read, and the mutation test rejects widening it.
  • Docker-auth concern: both final-head OpenClaw jobs passed mode-specific auth setup, authentication, test, cleanup, and upload in order. The validator pins the same mode-specific path in setup, cleanup, and artifact isolation. Downloaded artifacts have no credential-shaped matches.
  • PRA-15/T1-T8: covered in proportion to the change: 13 focused retry/workflow/caller tests, 417 support tests, both typechecks, the disjoint-project check, 31 green PR checks, four green live cells, four mode-separated artifact roots, zero cleanup failures, and no timed-out commands.

No code change is needed for the remaining advisor items. They are either explicitly justified safety decisions, runtime follow-ups now completed, or pre-existing/out-of-scope observations.

@cv
cv marked this pull request as ready for review June 29, 2026 03:05
@cv

cv commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@test/e2e-scenario/fixtures/inference-switch-retry.ts`:
- Around line 22-39: The shared retry logic in runInferenceSetWithRetry is only
using isTransientInferenceSetFailure(), so OpenClaw’s provider-validation
429/quota cases are no longer treated as retryable. Update the retry decision in
runInferenceSetWithRetry to also recognize the NVIDIA Endpoints validation
failure patterns that openclaw-inference-switch.test.ts checks for, by combining
the existing classifier with the provider-validation classifier on the
ShellProbeResult output before deciding to continue retrying or fall back to the
unverified attempt.

In `@test/e2e-scenario/live/hermes-inference-switch-helpers.ts`:
- Around line 132-138: The cleanup step hardcodes the gateway name instead of
using the configured value, which can leave the real gateway registration behind
on non-default runs. Update the `sandbox.openshell` destroy call in
`hermes-inference-switch-helpers.ts` to use the same gateway name source
returned by `env()` / `OPENSHELL_GATEWAY`, matching the value used elsewhere in
this file, so cleanup always targets the active gateway.
🪄 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: CHILL

Plan: Enterprise

Run ID: bc27a192-59dc-4448-b5e9-15802f23d6a9

📥 Commits

Reviewing files that changed from the base of the PR and between 87058e0 and aa45910.

📒 Files selected for processing (10)
  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/fixtures/inference-switch-retry.ts
  • test/e2e-scenario/live/hermes-inference-switch-helpers.ts
  • test/e2e-scenario/live/hermes-inference-switch.test.ts
  • test/e2e-scenario/live/openclaw-inference-switch.test.ts
  • test/e2e-scenario/support-tests/hermes-inference-switch-command-shape.test.ts
  • test/e2e-scenario/support-tests/inference-switch-retry.test.ts
  • test/e2e-scenario/support-tests/inference-switch-workflow-boundary.test.ts
  • tools/e2e-scenarios/inference-switch-workflow-boundary.mts
  • tools/e2e-scenarios/workflow-boundary.mts

Comment thread test/e2e-scenario/fixtures/inference-switch-retry.ts
Comment thread test/e2e-scenario/live/hermes-inference-switch-helpers.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28346584299
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 1 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure
openclaw-inference-switch-vitest ✅ success

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28347337176
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 1 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure
openclaw-inference-switch-vitest ✅ success

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ❌ Some jobs failed

Run: 28348162532
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 1 passed, 1 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ❌ failure
openclaw-inference-switch-vitest ✅ success

Failed jobs: hermes-inference-switch-vitest. Check run artifacts for logs.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28382985940
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ✅ success
openclaw-inference-switch-vitest ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28383771048
Workflow ref: codex/e2e-anthropic-switch-matrix
Requested scenarios: (default — all supported)
Requested jobs: hermes-inference-switch-vitest,openclaw-inference-switch-vitest
Summary: 2 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-inference-switch-vitest ✅ success
openclaw-inference-switch-vitest ✅ success

@cv

cv commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Final exact-head investigation and evidence:

  • Run 28346584299 confirmed the configured-gateway cleanup finding was valid, but Hermes/Anthropic still failed after three verified endpoint checks and a successful bounded --no-verify route update.
  • Run 28347337176 proved the current nemoclaw destroy --yes --cleanup-gateway lifecycle correctly stopped the host gateway, but that cleanup correction was not sufficient to restore the migrated scenario.
  • Run 28348162532 falsified the loopback-endpoint hypothesis: verification could reach loopback, but the sandbox runtime could not. That experiment was reverted.
  • Comparing the frozen v0.0.69 shell test exposed the actual migration omissions: the direct Anthropic probe used max_tokens: 32, and both inference.local and Hermes API PONG boundaries had three-attempt, 5-second live retries. Restoring those semantics produced four green cells in run 28382985940. Its artifacts showed that the added firewall/preflight machinery had not made endpoint verification succeed; the restored bounded live retry was what crossed the eventually-consistent route boundary.
  • Commit 9bebd96bb therefore removes that unused preflight complexity while retaining only evidence-backed parity: host-alias routing, the frozen 32-token direct payload, bounded live PONG retries, and CLI-owned gateway cleanup.

Final exact-head run 28383771048 at GitHub-verified head 9bebd96bba7c3748ad61ff60d2690b0cd7df90dc passed all four cells. Downloaded artifacts show four isolated agent/mode roots, zero cleanup failures, no timed-out command results, no credential-shaped matches, three transient verified Hermes/Anthropic failures followed by the bounded zero-exit --no-verify update, a direct Anthropic PONG using max_tokens: 32, and a Hermes API PONG. All non-skipped PR checks are green.

The four #5919 inventory rows remain deliberately unchecked: merge, paired frozen-v0.0.69 execution, and independent semantic review are still required before an Equivalent disposition is final.

@cv
cv merged commit bf64f4e into main Jun 29, 2026
123 checks passed
@cv
cv deleted the codex/e2e-anthropic-switch-matrix branch June 29, 2026 15:52
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Advance the default Hermes final-image base to the newly published
post-stale-layout digest and retire the temporary digest-coupled repair
that intentionally rejected newer published bases. This restores Hermes
onboarding and live E2E builds after the `latest` base moved, while
keeping current-state fail-closed layout checks and the older
`.hermes-data` compatibility migration.

## Related Issue

Unblocks NVIDIA#5947. Follow-up to NVIDIA#5882.

## Changes

- Pin the default Hermes base to
`sha256:8dad3b989a9ed1e601743310b97be21be5f59f89f7913a47d04f3ec3c40b8ce6`,
whose published ARM64 image has neither `/sandbox/.openclaw` nor
`/sandbox/.hermes-data`.
- Remove the fired `NEMOCLAW_STALE_OPENCLAW_BASE_DIGEST` guard and
temporary `/sandbox/.openclaw` repair; replace it with a small
fail-closed invariant that rejects retired OpenClaw state rather than
maintaining it.
- Keep published-image provenance in the resolver: export immutable
official GHCR digests and reject candidates containing `.openclaw` or
`.hermes-data` state, while preserving local rebuild and caller-selected
base refs in the final Dockerfile.
- Delete the dedicated stale-layout verifier job, script, helpers, and
tests now that their documented removal trigger has fired.
- Add a focused final-image layout suite covering OpenClaw-state
refusal, the retained `.hermes-data` migration, and symlink refusal;
keep immutable published-ref coverage at the resolver boundary and
extend live secret-boundary inspection to both retired paths.
- Reject nested symlinks before copying retained `.hermes-data`
compatibility state so migration cannot preserve links outside the
legacy tree.

## Type of Change

- [x] 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
<!-- 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:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: internal published-image
lifecycle cleanup; CLI behavior, configuration, supported workflows, and
the documented Hermes state location are unchanged.
- [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: pending independent
review of the Hermes sandbox image boundary.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## 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
- [ ] 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)
- [ ] 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)

Verification evidence:

- Pulled and inspected the published ARM64 `8dad…` image;
`/sandbox/.openclaw` and `/sandbox/.hermes-data` are absent.
- Built `agents/hermes/Dockerfile` against the exact `8dad…` digest and
verified sandbox-user readability, runtime permissions, runtime
symlinks, and absence of both stale paths.
- Focused integration run passed 61 tests across
`hermes-final-image-layout`, `hermes-doctor-config-hash`,
`pr-workflow-contract`, and `sandbox-provisioning`.
- `npm run build:cli`, `npm run typecheck:cli`, `npm run checks`, and
`npm run test:projects:check` passed.
- Normal commit and push hooks passed for all commits, including the
full CLI test lane, shellcheck, hadolint, repository checks,
source-shape and test-size budgets, gitleaks, commitlint, and CLI
TypeScript checks.
- [Vitest E2E run
28339846140](https://github.com/NVIDIA/NemoClaw/actions/runs/28339846140)
passed Hermes install/inference, root entrypoint, and secret-boundary
jobs, then exposed that Dockerfile-level registry validation rejected
the forced local `:latest` base-cache rebuild. Commit `ce85e762a` moves
provenance back to the resolver boundary; [exact-head rerun
28340627350](https://github.com/NVIDIA/NemoClaw/actions/runs/28340627350)
passed all four jobs, including stale-base rebuild.
- Downloaded exact-head artifacts have no credential-shaped matches;
every command result in the rebuild artifact is zero, post-rebuild
inference returned `PONG`, backup scanning found no leaks, and all
scenario cleanups report no failures.
- Required documentation writer assessment, rerun after the fail-closed
review fixes, reported no user-facing docs changes needed.

---
<!-- 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>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Restores the two v0.0.69 Anthropic inference-switch workflow variants as
distinct Vitest matrix executions. The existing OpenClaw and Hermes live
tests already implement the compatible Anthropic provider, mock
protocol, route/config mutation, `inference.local`, and agent
assertions; this PR makes that mode unavoidable in default and selected
CI runs.

## Related Issue

Refs NVIDIA#5919

## Changes

- run hosted-compatible and Anthropic-compatible legs for both
inference-switch jobs with `fail-fast: false`
- preserve the old Anthropic provider, API, model, mock, agent, and
sandbox-name mode values
- separate artifacts by agent and mode so each execution has reviewable
evidence
- limit Docker credentials to the OpenClaw live-test window and remove
them before artifact upload
- add a focused workflow-boundary ratchet with mutations for removed
modes, hard-coded mode mappings, and credential-cleanup ordering
- restore the v0.0.69 Hermes gateway-reset lifecycle boundary before
isolated install attempts
- share the v0.0.69 inference-switch retry policy across Hermes and
OpenClaw: verified attempts first, `--no-verify` only after bounded
transient failures, then mandatory live route/config/inference/agent
assertions

## Type of Change

- [x] 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

<!-- 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:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: CI execution-mode restoration
and test-harness parity; no production CLI, configuration, or
user-workflow change. Required docs-writer reassessment after the final
fix confirmed no docs changes.
- [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: pending independent
human review; CodeQL, CodeRabbit, both PR advisors, the credential scan,
and exact live matrix evidence are recorded below.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## 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
- [ ] 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)
- [ ] 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)

Verification evidence:

- `e2e-vitest-support`: 57 files / 417 tests passed with 16 workers.
- Focused inference-switch support suite: 14 tests passed, covering
workflow mode ratchets, Hermes command/lifecycle wiring, verified
transient retries, fallback only after retry exhaustion, no fallback for
non-transient failures, and attempt-count validation.
- `npm run typecheck`, `npm run typecheck:cli`, Biome on changed
TypeScript, and `npm run test:projects:check` passed; Vitest reports 903
files across 8 disjoint projects.
- Normal commit and push hooks passed, including the broad CLI
coverage/ratchet lane, source-shape and test-size budgets, repository
checks, secret scanning, commitlint, and CLI TypeScript checks.
- All non-skipped PR checks pass at final head
`9bebd96bba7c3748ad61ff60d2690b0cd7df90dc`.
- [Exact-head live run
28383771048](https://github.com/NVIDIA/NemoClaw/actions/runs/28383771048)
passed all four cells: Hermes hosted, Hermes Anthropic-compatible,
OpenClaw hosted, and OpenClaw Anthropic-compatible.
- Downloaded artifacts contain four distinct agent/mode roots. Both
OpenClaw `scenario-result.json` files report `passed` with route,
config, registry/session, `inference.local`, and agent PONG assertions
true. Both Hermes modes have zero-exit route, config/hash,
`inference.local`, API-chat, and primary destroy results; both chat
boundaries returned PONG.
- Hermes Anthropic exhausted three transient host-side
endpoint-verification attempts, used the bounded `--no-verify` fallback,
then passed the independent OpenShell route, Hermes config/hash,
registry/session, sandbox `inference.local`, and Hermes API checks. The
direct Anthropic probe used the frozen `max_tokens: 32` payload and
returned PONG on its first bounded live attempt; the Hermes API boundary
also returned PONG. This is the same safety boundary as the frozen
v0.0.69 shared shell helper.
- Both OpenClaw jobs completed isolated Docker-auth setup,
authentication, live test, Docker-auth cleanup, then artifact upload in
that order.
- All four cleanup reports have zero failures; no command result timed
out; the downloaded artifacts contain no NVIDIA/GitHub/Slack/AWS/OpenAI
credential-shaped matches.
- Required documentation-writer assessment, rerun after the final
shared-helper fix, found no user-facing docs changes.

---
<!-- 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>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Expanded inference-switch e2e scenarios to run in both hosted and
Anthropic-compatible modes, with mode-aware artifact naming/paths.

* **Bug Fixes**
* Improved e2e reliability by adding shared retry orchestration for
inference-switch and Hermes PONG checks, retrying transient failures and
applying `--no-verify` only when the retry budget is exhausted.
* Refined Hermes and OpenClaw lifecycle orchestration (setup, install,
and cleanup) for more consistent sandbox/gateway handling.

* **Tests / Validation**
* Added end-to-end retry coverage and strengthened workflow boundary
validation, including matrix constraints and mode-scoped Docker auth
cleanup behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance provider: anthropic Anthropic or Claude provider behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants