Skip to content

fix(inference): inject OpenRouter runtime headers via adapter#6584

Merged
cv merged 14 commits into
mainfrom
feat/openrouter-runtime-headers
Jul 10, 2026
Merged

fix(inference): inject OpenRouter runtime headers via adapter#6584
cv merged 14 commits into
mainfrom
feat/openrouter-runtime-headers

Conversation

@sandl99

@sandl99 sandl99 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

OpenRouter runtime traffic now goes through a host-local NemoClaw adapter so runtime requests include the OpenRouter attribution headers that OpenShell cannot inject per provider today. The adapter keeps OPENROUTER_API_KEY owned by OpenShell, binds runtime requests to the exact OpenShell Authorization value via a persisted SHA-256 hash, and adds only HTTP-Referer plus X-OpenRouter-Title.
This runtime adapter is a workaround until OpenShell exposes L7 middleware/default-header injection for provider routes.

Related Issue

Fixes #5826.

Changes

  • Added an OpenRouter runtime adapter with split server, forwarding, lifecycle, and compiled TypeScript entrypoint modules that forward /v1/chat/completions to OpenRouter with attribution headers.
  • Wired openrouter-api onboarding to register OPENAI_BASE_URL=http://host.openshell.internal:11437/v1 while retaining OPENROUTER_API_KEY as the OpenShell provider credential.
  • Added NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT with gateway and adapter-port conflict validation.
  • Hardened adapter lifecycle with a startup lock/post-persist health probe, bounded request-body forwarding, exact bearer-token matching, and hash-only adapter state.
  • Kept OpenRouter on the same featured cloud model picker/list as NVIDIA Endpoints while preserving OpenRouter-specific credential and manual-model validation.
  • Moved OpenRouter adapter uninstall cleanup into a focused module instead of growing the uninstall run-plan monolith.
  • Updated OpenRouter onboarding, adapter, provider recovery, port, and docs coverage.

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:
  • 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: Local review completed for the credential boundary. OpenShell still owns and injects OPENROUTER_API_KEY; the adapter process receives only a SHA-256 authorization hash and does not receive the key in its environment, state file, or token file. Adapter tests cover exact bearer-auth enforcement, path containment, safe health output, oversized-body rejection, generic upstream failure responses, and OpenRouter header forwarding. The adapter intentionally binds 0.0.0.0 because the current OpenShell container route reaches the host through host.openshell.internal; exact bearer matching narrows runtime access to the OpenShell-held credential value, and replacing the adapter with OpenShell L7/default-header middleware is tracked as the TODO in src/lib/onboard/openrouter-runtime.ts.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/core/ports.test.ts src/lib/inference/openrouter-runtime-adapter.test.ts src/lib/onboard/inference-providers/remote-openai-surface.test.ts src/lib/onboard/gateway-recovery.test.ts src/lib/onboard/recovered-provider-reuse.test.ts src/lib/inference/config.test.ts src/lib/inference/onboard-probes.test.ts src/lib/onboard/providers.test.ts src/lib/onboard/setup-nim-flow.test.ts src/lib/onboard/setup-inference-route-containment.test.ts passed; npx vitest run --project integration test/onboard-openrouter-inference.test.ts passed; npm run typecheck:cli passed; npm run build:cli passed; npm run source-shape:check passed; npm run test-size:check passed. Follow-up hardening: npx vitest run --project cli src/lib/onboard/setup-nim-flow.test.ts src/lib/inference/nvidia-featured-models.test.ts src/lib/onboard/nvidia-featured-model-selection.test.ts src/lib/inference/openrouter-runtime-adapter.test.ts src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.test.ts src/lib/actions/uninstall/run-plan.test.ts passed; npx vitest run --project integration test/onboard-openrouter-inference.test.ts passed; npm run typecheck:cli passed; npm run build:cli passed; npm run docs passed; npm run test-size:check passed; npm run test-conditionals:scan -- --top 25 passed; npx tsx scripts/check-env-var-docs.ts passed; git diff --check passed. Upstream-abort regression follow-up: npx vitest run --project cli src/lib/inference/openrouter-runtime-adapter.test.ts src/lib/onboard/inference-providers/remote-openai-surface.test.ts src/lib/onboard/setup-nim-flow.test.ts src/lib/inference/nvidia-featured-models.test.ts src/lib/onboard/nvidia-featured-model-selection.test.ts src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.test.ts src/lib/actions/uninstall/run-plan.test.ts passed; npx vitest run --project integration test/onboard-openrouter-inference.test.ts passed; npm run typecheck:cli passed; npm run build:cli passed; npm run source-shape:check passed; npm run test-size:check passed; git diff --check passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • New Features
    • Added an OpenRouter runtime adapter with a public /health endpoint and forwarding for POST /v1/chat/completions only, including OpenRouter attribution headers.
    • Onboarding can configure an OpenRouter-backed inference route and support the new runtime adapter port/port offset.
  • Bug Fixes
    • Improved adapter request handling with body size enforcement and explicit auth/endpoint gating.
    • Strengthened port validation to reserve the OpenRouter adapter port and prevent gateway conflicts (including recovery).
  • Documentation
    • Documented NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT, validation behavior, and runtime adapter health/log details.
  • Tests
    • Added adapter forwarding/auth gating/request-size tests and expanded port validation coverage.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this Jul 9, 2026
@github-code-quality

github-code-quality Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the branch.

TypeScript / code-coverage/cli

The overall coverage in the branch remains at 77%, unchanged from the branch.

Show a code coverage summary of the most impacted files.
File 9dec1fb ec29c65 +/-
src/lib/actions...ge-preflight.ts 89% 74% -15%
src/lib/state/config-io.ts 88% 95% +7%
src/lib/actions...e-validation.ts 81% 90% +9%
src/lib/actions...x/mcp-bridge.ts 35% 44% +9%
src/lib/inferen...er-lifecycle.ts 0% 11% +11%
src/lib/inferen...apter-server.ts 0% 67% +67%
src/lib/onboard...uter-runtime.ts 0% 70% +70%
src/lib/actions...pter-cleanup.ts 0% 85% +85%
src/lib/inferen...pter-forward.ts 0% 88% +88%
src/lib/inferen...apter-common.ts 0% 88% +88%

Updated July 10, 2026 05:03 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@coderabbitai

coderabbitai Bot commented Jul 9, 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

Adds an OpenRouter runtime adapter, dedicated port validation, onboarding wiring, and updated docs/tests for the adapter path and its validation rules.

Changes

OpenRouter runtime adapter feature

Layer / File(s) Summary
OpenRouter adapter port configuration and validation
src/lib/core/ports.ts, src/lib/core/ports.test.ts, docs/inference/inference-options.mdx, docs/reference/commands.mdx
Adds OPENROUTER_RUNTIME_ADAPTER_PORT, extends port-conflict checks, and updates the documented OpenRouter adapter and validation rules.
Runtime adapter HTTP server and process lifecycle
src/lib/inference/openrouter-runtime-adapter-common.ts, src/lib/inference/openrouter-runtime-adapter-forward.ts, src/lib/inference/openrouter-runtime-adapter-lifecycle.ts, src/lib/inference/openrouter-runtime-adapter-server.ts, src/lib/inference/openrouter-runtime-adapter-entry.ts, src/lib/inference/openrouter-runtime-adapter.ts, src/lib/inference/openrouter-runtime-adapter.test.ts
Implements adapter logging, forwarding, lifecycle management, HTTP handling, entrypoint wiring, exports, and runtime adapter tests.
Adapter base URLs and gateway recovery wiring
src/lib/inference/openrouter.ts, src/lib/onboard/gateway-recovery.ts, src/lib/onboard/gateway-recovery.test.ts
Adds adapter-derived base URLs and includes the adapter port in gateway recovery validation.
Onboarding orchestration for OpenRouter runtime setup
src/lib/onboard/openrouter-runtime.ts
Adds the onboarding path that starts or reuses the adapter, configures the provider route, and performs verification.
Dependency wiring into setup-inference and remote provider flow
src/lib/onboard/inference-providers/types.ts, src/lib/onboard/inference-providers/remote.ts, src/lib/onboard/setup-inference.ts, src/lib/onboard.ts, test/support/setup-inference-test-harness.ts, src/lib/onboard/inference-providers/remote-openai-surface.test.ts, src/lib/onboard/setup-inference-route-containment.test.ts, test/onboard-openrouter-inference.test.ts
Threads the OpenRouter onboarding dependency and smoke-skip flag through setup inference and updates the related test harnesses and onboarding tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Onboarding as setupOpenRouterRuntimeInference
  participant Lifecycle as ensureOpenRouterRuntimeAdapter
  participant Adapter as OpenRouterRuntimeAdapterServer
  participant OpenShell
  participant OpenRouter as openrouter.ai

  Onboarding->>Lifecycle: ensureOpenRouterRuntimeAdapter()
  Lifecycle->>Adapter: spawn/reuse detached process
  Lifecycle-->>Onboarding: adapter route (baseUrl, logPath)
  Onboarding->>OpenShell: upsertProvider(baseUrl) + inference set
  Onboarding->>Adapter: verifyOnboardInferenceSmoke request
  Adapter->>OpenRouter: forward chat completions with attribution headers
  OpenRouter-->>Adapter: response
  Adapter-->>Onboarding: smoke verification result
Loading

Suggested labels: bug-fix, area: onboarding

Suggested reviewers: cv, jyaunches

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.13% 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
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.
Title check ✅ Passed The title accurately summarizes the core change: adding an OpenRouter runtime adapter to inject headers during inference traffic.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/openrouter-runtime-headers

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

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Optional E2E: cloud-inference, full-e2e

Dispatch hint: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard: Required by the deterministic risk plan for ci/platform-matrix.json and because onboarding/provider changes can affect full hosted onboarding on a clean supported host.
  • inference-routing: Required by the deterministic risk plan because OpenRouter provider setup, runtime adapter lifecycle, forwarding, model/catalog selection, and OpenShell provider registration can affect the real inference route advertised to the agent.
  • network-policy: Required by the deterministic risk plan because the new OpenRouter runtime adapter changes the sandbox-to-host-to-provider inference path and reserved port behavior, which must be validated at the real network-policy boundary.
  • onboard-repair: Required by the deterministic risk plan and onboarding resume rule. Changes in onboarding, gateway recovery, setup-inference, and provider state can affect repair convergence after partial failures or stale runtime state.
  • onboard-resume: Required by the deterministic risk plan and onboarding resume rule. Changes in onboarding, gateway recovery, setup-inference, and provider state can affect persisted resume state and re-entry into the live onboarding flow.

Optional E2E

  • cloud-inference: Optional adjacent confidence for hosted provider model selection and runtime inference behavior after changes to featured model selection and OpenRouter/NVIDIA catalog handling.
  • full-e2e: Optional broad user-flow smoke for the default OpenClaw assistant after onboarding and inference setup changes.

New E2E recommendations

  • openrouter-runtime-adapter (high): The existing required inference-routing and network-policy jobs validate generic route behavior, but there is no dedicated live OpenRouter runtime-adapter E2E that proves the host adapter starts, injects OpenRouter attribution headers, enforces the bound Authorization token, exposes health, writes logs, and is the base URL registered into OpenShell.
    • Suggested test: Add a live OpenRouter runtime adapter E2E using a local fake OpenRouter-compatible endpoint and an OpenShell sandbox route through host.openshell.internal:.
  • uninstall-cleanup (medium): The PR adds host process and state-file cleanup for the OpenRouter runtime adapter, but the repository appears to cover this with unit tests rather than a live uninstall E2E proving an installed/onboarded adapter process is stopped without killing a foreign process on the same port pattern.
    • Suggested test: Add an uninstall cleanup E2E that starts a managed OpenRouter runtime adapter during onboarding, runs nemoclaw uninstall, and verifies adapter PID/state/log cleanup while preserving unrelated host listeners.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=inference-routing
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=network-policy
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • cloud-onboard: Installer and platform changes must work on a clean supported host with the pinned runtime dependencies.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • inference-routing: Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=inference-routing
  • network-policy: Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=network-policy
  • onboard-repair: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
  • onboard-resume: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume

Optional E2E targets

  • None.

Relevant changed files

  • ci/platform-matrix.json
  • src/lib/core/ports.ts
  • src/lib/inference/openrouter-runtime-adapter-common.ts
  • src/lib/inference/openrouter-runtime-adapter-entry.ts
  • src/lib/inference/openrouter-runtime-adapter-forward.ts
  • src/lib/inference/openrouter-runtime-adapter-lifecycle.ts
  • src/lib/inference/openrouter-runtime-adapter-server.ts
  • src/lib/inference/openrouter-runtime-adapter.ts
  • src/lib/inference/openrouter.ts
  • src/lib/onboard.ts
  • src/lib/onboard/gateway-recovery.ts
  • src/lib/onboard/inference-providers/remote.ts
  • src/lib/onboard/inference-providers/types.ts
  • src/lib/onboard/openrouter-runtime.ts
  • src/lib/onboard/setup-inference.ts
  • src/lib/onboard/setup-nim-flow.ts
  • src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts
  • src/lib/actions/uninstall/run-plan.ts

@sandl99 sandl99 added area: inference Inference routing, serving, model selection, or outputs area: providers Inference provider integrations and provider behavior enhancement New capability or improvement request labels Jul 9, 2026
Comment thread src/lib/inference/openrouter-runtime-adapter.ts Fixed
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-6: Authorization hash persisted to disk violates 'nothing written to disk' acceptance and enables offline attacks; then add or justify PRA-T1.
Open items: 1 required · 21 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 11 still apply · 12 new items found

Action checklist

  • PRA-6 Fix: Authorization hash persisted to disk violates 'nothing written to disk' acceptance and enables offline attacks in src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225
  • PRA-1 Resolve or justify: Monolith growth in uninstall run-plan test file in src/lib/actions/uninstall/run-plan.test.ts:1437
  • PRA-2 Resolve or justify: Monolith growth in uninstall run-plan implementation in src/lib/actions/uninstall/run-plan.ts:992
  • PRA-3 Resolve or justify: Monolith growth in setup-inference orchestration in src/lib/onboard/setup-inference.ts:450
  • PRA-4 Resolve or justify: Duplicate port resolution logic; should import from ports.ts in src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:29
  • PRA-5 Resolve or justify: validateOpenRouterRuntimeAdapterPort duplicates gateway port validation logic in src/lib/core/ports.ts:140
  • PRA-7 Resolve or justify: Adapter binds 0.0.0.0 instead of loopback, expanding attack surface in src/lib/inference/openrouter.ts:18
  • PRA-8 Resolve or justify: Multiple silent error handling paths lose observability for security events in src/lib/inference/openrouter-runtime-adapter-common.ts:48
  • PRA-9 Resolve or justify: Adapter port validation missing Hermes port 8642 conflict check in src/lib/core/ports.ts:156
  • PRA-10 Resolve or justify: Runtime adapter workaround for missing OpenShell L7 middleware lacks source fix plan in src/lib/onboard/openrouter-runtime.ts:10
  • PRA-11 Resolve or justify: Missing test: removeStaleLock non-ENOENT errors logged and do not block startup in src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:79
  • 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: Missing test: removeStaleLock non-ENOENT errors logged and do not block startup
  • PRA-T7 Add or justify test follow-up: Missing test: lock cleanup failure logged and does not permanently block startup
  • PRA-T8 Add or justify test follow-up: Missing test: probeAdapterHealth classifies failure types (timeout vs address error)

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture src/lib/actions/uninstall/run-plan.test.ts:1437 Extract OpenRouter adapter cleanup test setup into a dedicated test helper module (e.g., test/support/openrouter-adapter-test-setup.ts) and import it here.
PRA-2 Resolve/justify architecture src/lib/actions/uninstall/run-plan.ts:992 Consider a registry pattern where adapter cleanup functions self-register, or a dedicated cleanup orchestration module that collects all adapter stoppers.
PRA-3 Resolve/justify architecture src/lib/onboard/setup-inference.ts:450 Extract provider-specific onboarding into a strategy/registry pattern (e.g., InferenceProviderSetupStrategy interface) so each provider encapsulates its adapter setup, validation, and registration.
PRA-4 Resolve/justify architecture src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:29 Import OPENROUTER_RUNTIME_ADAPTER_PORT from ../core/ports and use it directly. Remove resolveOpenRouterRuntimeAdapterPort and DEFAULT_OPENROUTER_RUNTIME_ADAPTER_PORT.
PRA-5 Resolve/justify architecture src/lib/core/ports.ts:140 Extract shared port conflict validation into a common internal function used by both validateGatewayPort and validateOpenRouterRuntimeAdapterPort.
PRA-6 Required security src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225 Either (a) update issue #5826 to acknowledge hash persistence as deliberate tradeoff for adapter restart survival, or (b) redesign to keep hash only in memory (require re-auth on adapter restart). Option (a) with explicit issue update is pragmatic given PR body documents this boundary.
PRA-7 Resolve/justify security src/lib/inference/openrouter.ts:18 Change OPENROUTER_RUNTIME_ADAPTER_BIND_HOST to '127.0.0.1' since host.openshell.internal resolves to loopback. If container networking requires 0.0.0.0, document the justification and add network policy to restrict access.
PRA-8 Resolve/justify security src/lib/inference/openrouter-runtime-adapter-common.ts:48 Propagate log write errors via metric/side-channel. Log lock cleanup failures. Classify health probe failures (timeout/connection refused vs address/route errors). Classify startup failures with distinct exit codes. Log PID file read failures before falling back to lsof scan.
PRA-9 Resolve/justify security src/lib/core/ports.ts:156 Add { label: 'Hermes OpenAI-compatible API', port: 8642 } to reservedDefaults in validateOpenRouterRuntimeAdapterPort.
PRA-10 Resolve/justify security src/lib/onboard/openrouter-runtime.ts:10 Link to upstream OpenShell issue, add regression test that fails when OpenShell gains L7 middleware/header injection (proving adapter removable), and document removal criteria.
PRA-11 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:79 Add test: simulate lock file statSync failure with EACCES, verify adapter startup proceeds and error is logged.
PRA-12 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:95 Add test: simulate lock file that cannot be unlinked, verify cleanup failure logged and subsequent startup attempts eventually succeed.
PRA-13 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:173 Add test: probeAdapterHealth against invalid bind address returns false with logged address error; against refused connection returns false with logged connection error.
PRA-14 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-entry.ts:8 Add tests for each classified startup failure mode with expected exit codes.
PRA-15 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-common.ts:48 Add test: simulate log write failure (readonly fs), verify failure reported via metric or side-channel.
PRA-16 Resolve/justify tests src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:118 Add test: corrupt PID file + missing lsof -> warning logged and alternative cleanup attempted.
PRA-17 Resolve/justify tests src/lib/core/ports.test.ts:162 Add test: validateOpenRouterRuntimeAdapterPort rejects 8642 with Hermes label.
PRA-18 Resolve/justify tests src/lib/onboard/openrouter-runtime.ts:10 Add test that fails (or detects capability) when OpenShell exposes L7 middleware/default-header injection for provider routes.
PRA-19 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225 Add test: ensureOpenRouterRuntimeAdapterLocked cleans up state file when health probe fails post-write.
PRA-20 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:294 Add test: start adapter, kill process leaving PID/state, restart adapter -> prior state cleaned up, new adapter healthy.

🚨 Required before merge

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

PRA-6 Required — Authorization hash persisted to disk violates 'nothing written to disk' acceptance and enables offline attacks

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225
  • Category: security
  • Problem: Issue feat: adding OpenRouter as an inference provider #5826 explicitly requires 'nothing is written to disk' for the OpenRouter API key. The adapter writes a SHA-256 hash of the key (authorizationHash) to ~/.nemoclaw/openrouter-runtime-adapter.json with 0o600 permissions. While the raw key is not stored, the persisted hash enables offline verification attacks against guessed API keys and remains on disk after onboarding.
  • Impact: Credential-derived secret persisted contrary to acceptance criteria; offline brute-force verification of API keys possible; file persists after onboarding.
  • Required action: Either (a) update issue feat: adding OpenRouter as an inference provider #5826 to acknowledge hash persistence as deliberate tradeoff for adapter restart survival, or (b) redesign to keep hash only in memory (require re-auth on adapter restart). Option (a) with explicit issue update is pragmatic given PR body documents this boundary.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run onboarding with OpenRouter, inspect ~/.nemoclaw/openrouter-runtime-adapter.json for authorizationHash field (64-char hex).
  • Missing regression test: Add test verifying adapter state file contains authorizationHash after onboarding; document as expected behavior per updated acceptance.
  • Done when: The required change is committed and verification passes: Run onboarding with OpenRouter, inspect ~/.nemoclaw/openrouter-runtime-adapter.json for authorizationHash field (64-char hex).
  • Evidence: lifecycle.ts:225-230 writeLocalAdapterJsonFile includes authorizationHash common.ts:77-80 adapterAuthorizationHash creates SHA-256 of token Issue feat: adding OpenRouter as an inference provider #5826 body: 'nothing is written to disk' inference-options.mdx:96 claims 'adapter does not store the API key' but omits hash
Review findings by urgency: 1 required fix, 21 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 — Monolith growth in uninstall run-plan test file

  • Location: src/lib/actions/uninstall/run-plan.test.ts:1437
  • Category: architecture
  • Problem: Test file grew by 8 lines (1437→1445) adding OpenRouter adapter cleanup test setup inline. Continued provider-specific test fixture accumulation increases maintenance burden.
  • Impact: Test file becomes harder to maintain and navigate as provider-specific cleanup logic accumulates linearly.
  • Recommended action: Extract OpenRouter adapter cleanup test setup into a dedicated test helper module (e.g., test/support/openrouter-adapter-test-setup.ts) and import it here.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line count of src/lib/actions/uninstall/run-plan.test.ts and whether OpenRouter adapter test setup is inlined or extracted.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line count of src/lib/actions/uninstall/run-plan.test.ts and whether OpenRouter adapter test setup is inlined or extracted.
  • Evidence: Monolith delta: baseLines=1437, headLines=1445, delta=8, severity=warning Test adds openrouter-runtime-adapter.pid/json/lock/log file setup in setupStateDir()

PRA-2 Resolve/justify — Monolith growth in uninstall run-plan implementation

  • Location: src/lib/actions/uninstall/run-plan.ts:992
  • Category: architecture
  • Problem: Implementation file grew by 2 lines (992→994) adding import and call to stopOpenRouterRuntimeAdapter. Continued provider-specific cleanup logic accumulation increases cognitive load.
  • Impact: Uninstall plan execution accumulates provider-specific steps inline, making the function harder to audit for completeness and ordering.
  • Recommended action: Consider a registry pattern where adapter cleanup functions self-register, or a dedicated cleanup orchestration module that collects all adapter stoppers.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/actions/uninstall/run-plan.ts for inline adapter cleanup calls vs. a registry/orchestrator.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/actions/uninstall/run-plan.ts for inline adapter cleanup calls vs. a registry/orchestrator.
  • Evidence: Monolith delta: baseLines=992, headLines=994, delta=2, severity=warning Added import and call to stopOpenRouterRuntimeAdapter in executePlan()

PRA-3 Resolve/justify — Monolith growth in setup-inference orchestration

  • Location: src/lib/onboard/setup-inference.ts:450
  • Category: architecture
  • Problem: Orchestration file grew by 3 lines (450→453) adding OpenRouter provider handling. Continued provider-specific onboarding logic accumulation increases cognitive load.
  • Impact: Setup-inference accumulates provider-specific branching, making it harder to audit for completeness and maintain.
  • Recommended action: Extract provider-specific onboarding into a strategy/registry pattern (e.g., InferenceProviderSetupStrategy interface) so each provider encapsulates its adapter setup, validation, and registration.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/onboard/setup-inference.ts for provider-specific branching vs. a strategy/registry pattern.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/onboard/setup-inference.ts for provider-specific branching vs. a strategy/registry pattern.
  • Evidence: Monolith delta: baseLines=450, headLines=453, delta=3, severity=warning Added OpenRouter provider handling in setup-inference flow

PRA-4 Resolve/justify — Duplicate port resolution logic; should import from ports.ts

  • Location: src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:29
  • Category: architecture
  • Problem: cleanup.ts reimplements port parsing and validation logic (resolveOpenRouterRuntimeAdapterPort) that already exists as OPENROUTER_RUNTIME_ADAPTER_PORT constant and validation in ports.ts.
  • Impact: Drift risk: port default/validation changes in ports.ts won't propagate to cleanup logic, causing inconsistency.
  • Recommended action: Import OPENROUTER_RUNTIME_ADAPTER_PORT from ../core/ports and use it directly. Remove resolveOpenRouterRuntimeAdapterPort and DEFAULT_OPENROUTER_RUNTIME_ADAPTER_PORT.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts for resolveOpenRouterRuntimeAdapterPort vs. import from ports.ts.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts for resolveOpenRouterRuntimeAdapterPort vs. import from ports.ts.
  • Evidence: cleanup.ts:29-42 defines resolveOpenRouterRuntimeAdapterPort with same logic as ports.ts ports.ts exports OPENROUTER_RUNTIME_ADAPTER_PORT constant

PRA-5 Resolve/justify — validateOpenRouterRuntimeAdapterPort duplicates gateway port validation logic

  • Location: src/lib/core/ports.ts:140
  • Category: architecture
  • Problem: New validateOpenRouterRuntimeAdapterPort function replicates conflict-checking logic from validateGatewayPort instead of sharing a common validator.
  • Impact: Drift risk: port conflict rules updated in one validator but not the other, leading to inconsistent validation.
  • Recommended action: Extract shared port conflict validation into a common internal function used by both validateGatewayPort and validateOpenRouterRuntimeAdapterPort.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare validateGatewayPort and validateOpenRouterRuntimeAdapterPort conflict-checking logic in ports.ts.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare validateGatewayPort and validateOpenRouterRuntimeAdapterPort conflict-checking logic in ports.ts.
  • Evidence: ports.ts:140-185 validateOpenRouterRuntimeAdapterPort replicates reservedDefaults and conflicts arrays from validateGatewayPort Both functions check dashboard range, reserved defaults, and configured port conflicts

PRA-7 Resolve/justify — Adapter binds 0.0.0.0 instead of loopback, expanding attack surface

  • Location: src/lib/inference/openrouter.ts:18
  • Category: security
  • Problem: OPENROUTER_RUNTIME_ADAPTER_BIND_HOST is set to '0.0.0.0', causing the adapter to listen on all interfaces. Since sandbox traffic reaches the adapter via host.openshell.internal (which resolves to host loopback), binding 0.0.0.0 is unnecessary and expands the attack surface if an auth bypass is discovered.
  • Impact: Adapter accessible on all host interfaces instead of only loopback; increased attack surface for auth bypass or DoS.
  • Recommended action: Change OPENROUTER_RUNTIME_ADAPTER_BIND_HOST to '127.0.0.1' since host.openshell.internal resolves to loopback. If container networking requires 0.0.0.0, document the justification and add network policy to restrict access.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check openrouter.ts:18 for OPENROUTER_RUNTIME_ADAPTER_BIND_HOST value; verify host.openshell.internal resolves to 127.0.0.1 in sandbox.
  • Missing regression test: Add test that adapter binds only 127.0.0.1 (or document why 0.0.0.0 is required with network policy).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check openrouter.ts:18 for OPENROUTER_RUNTIME_ADAPTER_BIND_HOST value; verify host.openshell.internal resolves to 127.0.0.1 in sandbox.
  • Evidence: openrouter.ts:18 OPENROUTER_RUNTIME_ADAPTER_BIND_HOST = '0.0.0.0' openrouter.ts:21 OPENROUTER_RUNTIME_ADAPTER_LOOPBACK_HOST = '127.0.0.1' Sandbox route uses host.openshell.internal:11437

PRA-8 Resolve/justify — Multiple silent error handling paths lose observability for security events

  • Location: src/lib/inference/openrouter-runtime-adapter-common.ts:48
  • Category: security
  • Problem: defaultAdapterLogger and logAdapterEvent swallow log write failures (console.error only). removeStaleLock release fn swallows unlinkSync errors. probeAdapterHealth catches all errors -> false (masks bind/route/config errors). entry.ts top-level catch -> exit 1 (no failure classification). cleanup PID read swallows errors. These silent failures reduce visibility into adapter health and potential security events.
  • Impact: Operators cannot detect logging failures, lock cleanup failures, or distinguish health probe failure types; security-relevant events may go unnoticed.
  • Recommended action: Propagate log write errors via metric/side-channel. Log lock cleanup failures. Classify health probe failures (timeout/connection refused vs address/route errors). Classify startup failures with distinct exit codes. Log PID file read failures before falling back to lsof scan.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Simulate each silent failure (readonly fs for logs, immutable lock file, invalid bind address, missing env, corrupt PID file) and verify failures are logged/classified.
  • Missing regression test: Add tests for each silent failure path verifying structured logging/classification.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Simulate each silent failure (readonly fs for logs, immutable lock file, invalid bind address, missing env, corrupt PID file) and verify failures are logged/classified.
  • Evidence: common.ts:44-50 defaultAdapterLogger try/catch -> console.error only common.ts:55-63 logAdapterEvent try/catch -> console.error only lifecycle.ts:92-95 release fn bare catch lifecycle.ts:149-178 probeAdapterHealth catches all -> false entry.ts:3-9 try/catch -> exit 1 cleanup.ts:114-118 try/catch swallows PID read errors

PRA-9 Resolve/justify — Adapter port validation missing Hermes port 8642 conflict check

  • Location: src/lib/core/ports.ts:156
  • Category: security
  • Problem: validateOpenRouterRuntimeAdapterPort checks conflicts against gateway, dashboard range, vLLM (8000), Ollama (11434), Ollama proxy (11435), Bedrock adapter (11436), but omits HERMES_OPENAI_API_PORT (8642). validateGatewayPort includes Hermes port in reservedDefaults. OpenRouter adapter port could be set to 8642, conflicting with Hermes OpenAI-compatible API.
  • Impact: Port conflict between OpenRouter runtime adapter and Hermes API port, breaking Hermes onboarding or adapter startup.
  • Recommended action: Add { label: 'Hermes OpenAI-compatible API', port: 8642 } to reservedDefaults in validateOpenRouterRuntimeAdapterPort.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Set NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT=8642 and run port validation; should reject with Hermes conflict.
  • Missing regression test: Add test that validateOpenRouterRuntimeAdapterPort rejects 8642 with Hermes label.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Set NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT=8642 and run port validation; should reject with Hermes conflict.
  • Evidence: ports.ts:91 validateGatewayPort reservedDefaults includes Hermes port 8642 ports.ts:156-163 validateOpenRouterRuntimeAdapterPort reservedDefaults omits Hermes port

PRA-10 Resolve/justify — Runtime adapter workaround for missing OpenShell L7 middleware lacks source fix plan

  • Location: src/lib/onboard/openrouter-runtime.ts:10
  • Category: security
  • Problem: TODO comment acknowledges adapter is a workaround pending OpenShell L7 middleware/default-header injection, but no upstream issue link, regression test proving adapter still needed, or removal timeline. Workaround persists hash to disk (violating acceptance), binds 0.0.0.0 unnecessarily, and adds a new trusted-code component. Without a plan to remove it, this becomes permanent technical debt in the trust boundary.
  • Impact: Workaround may become permanent; no verification that OpenShell cannot be fixed; adapter adds attack surface and trust boundary complexity.
  • Recommended action: Link to upstream OpenShell issue, add regression test that fails when OpenShell gains L7 middleware/header injection (proving adapter removable), and document removal criteria.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check openrouter-runtime.ts:10 for TODO with upstream issue link, regression test, and removal criteria.
  • Missing regression test: Add test that fails when OpenShell gains L7 middleware/header injection capability, proving adapter can be removed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check openrouter-runtime.ts:10 for TODO with upstream issue link, regression test, and removal criteria.
  • Evidence: openrouter-runtime.ts:10-14 TODO documents workaround No upstream issue reference, regression test, or removal timeline Adapter persists hash to disk, binds 0.0.0.0, adds new trusted component

PRA-11 Resolve/justify — Missing test: removeStaleLock non-ENOENT errors logged and do not block startup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:79
  • Category: tests
  • Problem: removeStaleLock throws on non-ENOENT filesystem errors (e.g., EACCES). tryAcquireAdapterLock calls it without try/catch, so adapter startup can fail catastrophically instead of degrading gracefully. No test covers this error path.
  • Impact: Adapter startup may fail permanently on lock directory filesystem issues without test coverage proving graceful degradation.
  • Recommended action: Add test: simulate lock file statSync failure with EACCES, verify adapter startup proceeds and error is logged.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run test with mocked fs.statSync throwing EACCES; verify tryAcquireAdapterLock does not throw and logs error.
  • Missing regression test: Add test covering removeStaleLock non-ENOENT error handling in tryAcquireAdapterLock.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run test with mocked fs.statSync throwing EACCES; verify tryAcquireAdapterLock does not throw and logs error.
  • Evidence: lifecycle.ts:74-80 removeStaleLock throws on non-ENOENT lifecycle.ts:87 calls removeStaleLock without try/catch No existing test for this error path

PRA-12 Resolve/justify — Missing test: lock cleanup failure logged and does not permanently block startup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:95
  • Category: tests
  • Problem: Lock release function uses bare catch block (best-effort cleanup). If fs.unlinkSync fails (immutable file, permission change), lock persists indefinitely. No test verifies failure is logged and startup eventually succeeds.
  • Impact: Stale lock may permanently block adapter startup without detection.
  • Recommended action: Add test: simulate lock file that cannot be unlinked, verify cleanup failure logged and subsequent startup attempts eventually succeed.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Create immutable lock file, run adapter startup, verify warning logged and startup succeeds after lock TTL or manual removal.
  • Missing regression test: Add test for lock cleanup failure observability and non-permanent blocking.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Create immutable lock file, run adapter startup, verify warning logged and startup succeeds after lock TTL or manual removal.
  • Evidence: lifecycle.ts:92-95 release fn bare catch swallows all errors Comment acknowledges best-effort only No test for lock cleanup failure path

PRA-13 Resolve/justify — Missing test: probeAdapterHealth classifies failure types (timeout vs address error)

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:173
  • Category: tests
  • Problem: probeAdapterHealth catches all errors and returns false. No test verifies distinction between connection refused (adapter down) and address/route errors (config issue).
  • Impact: Operators cannot distinguish transient vs configuration failures from health probe results.
  • Recommended action: Add test: probeAdapterHealth against invalid bind address returns false with logged address error; against refused connection returns false with logged connection error.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run probeAdapterHealth with invalid hostname and with nothing listening; verify distinct log entries.
  • Missing regression test: Add test classifying health probe failure types with distinct logging.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run probeAdapterHealth with invalid hostname and with nothing listening; verify distinct log entries.
  • Evidence: lifecycle.ts:149-178 probeAdapterHealth catches all -> resolve(false) No error classification or logging in probe path No test for failure classification

PRA-14 Resolve/justify — Missing tests: startup failure classification with distinct exit codes

  • Location: src/lib/inference/openrouter-runtime-adapter-entry.ts:8
  • Category: tests
  • Problem: Entry point exits with code 1 for all failures. No tests for classified failures: port conflict (2), missing env (3), lock contention (4), upstream config (5).
  • Impact: Automation cannot programmatically distinguish startup failure modes.
  • Recommended action: Add tests for each classified startup failure mode with expected exit codes.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Simulate each failure mode and verify distinct exit codes.
  • Missing regression test: Add tests for exit codes 2 (port conflict), 3 (missing env), 4 (lock contention), 5 (upstream config).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Simulate each failure mode and verify distinct exit codes.
  • Evidence: entry.ts:3-9 try/catch -> process.exit(1) for all errors No failure classification logic No tests for classified exit codes

PRA-15 Resolve/justify — Missing test: log write failures reported via metric/side-channel

  • Location: src/lib/inference/openrouter-runtime-adapter-common.ts:48
  • Category: tests
  • Problem: defaultAdapterLogger and logAdapterEvent swallow log write failures (console.error only). No test verifies failures are reported beyond console.error.
  • Impact: Logging failures (disk full, permissions) go undetected by monitoring.
  • Recommended action: Add test: simulate log write failure (readonly fs), verify failure reported via metric or side-channel.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Mock appendLocalAdapterJsonLine to throw, call defaultAdapterLogger, verify side-channel metric incremented.
  • Missing regression test: Add test for log write failure observability beyond console.error.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Mock appendLocalAdapterJsonLine to throw, call defaultAdapterLogger, verify side-channel metric incremented.
  • Evidence: common.ts:44-50 defaultAdapterLogger try/catch -> console.error only common.ts:55-63 logAdapterEvent try/catch -> console.error only No side-channel/metric reporting

PRA-16 Resolve/justify — Missing test: corrupt PID file + missing lsof still attempts cleanup

  • Location: src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:118
  • Category: tests
  • Problem: PID file read errors silently ignored; fallback depends on lsof availability. No test covers corrupt PID file with lsof unavailable.
  • Impact: Orphan adapter process may persist if PID file unreadable and lsof unavailable.
  • Recommended action: Add test: corrupt PID file + missing lsof -> warning logged and alternative cleanup attempted.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Create corrupt PID file, mock commandExists('lsof') false, run cleanup, verify warning and fallback attempt.
  • Missing regression test: Add test for PID read failure + lsof unavailable cleanup path.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Create corrupt PID file, mock commandExists('lsof') false, run cleanup, verify warning and fallback attempt.
  • Evidence: cleanup.ts:114-118 try/catch swallows PID read errors cleanup.ts:122-125 lsof check with warn if missing No test for this combined failure scenario

PRA-17 Resolve/justify — Missing test: validateOpenRouterRuntimeAdapterPort rejects Hermes port 8642

  • Location: src/lib/core/ports.test.ts:162
  • Category: tests
  • Problem: Port validation for OpenRouter adapter omits Hermes port 8642 check (present in gateway validation). No test covers this conflict.
  • Impact: Port conflict between OpenRouter adapter and Hermes API possible without test detection.
  • Recommended action: Add test: validateOpenRouterRuntimeAdapterPort rejects 8642 with Hermes label.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run ports.test.ts with validateOpenRouterRuntimeAdapterPort(8642); expect throw with Hermes label.
  • Missing regression test: Add test case in ports.test.ts for Hermes port 8642 rejection.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run ports.test.ts with validateOpenRouterRuntimeAdapterPort(8642); expect throw with Hermes label.
  • Evidence: ports.ts:156-163 validateOpenRouterRuntimeAdapterPort reservedDefaults omits Hermes port ports.test.ts has no test for 8642 conflict PRA-12 and security finding F-010 identify this gap

PRA-18 Resolve/justify — Missing regression test: adapter removable when OpenShell gains L7 middleware

  • Location: src/lib/onboard/openrouter-runtime.ts:10
  • Category: tests
  • Problem: TODO acknowledges adapter is workaround pending OpenShell L7 middleware. No test fails when OpenShell gains this capability, proving adapter can be removed.
  • Impact: Workaround may become permanent technical debt without detection.
  • Recommended action: Add test that fails (or detects capability) when OpenShell exposes L7 middleware/default-header injection for provider routes.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check for OpenShell L7 middleware capability in test; fail if present (proving adapter removable).
  • Missing regression test: Add test detecting OpenShell L7 middleware capability as removal signal for adapter.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check for OpenShell L7 middleware capability in test; fail if present (proving adapter removable).
  • Evidence: openrouter-runtime.ts:10-14 TODO documents workaround No upstream issue, regression test, or removal criteria F-006 and F-011 require this test

PRA-19 Resolve/justify — Missing test: state file cleaned up when health probe fails after write

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225
  • Category: tests
  • Problem: If writeLocalAdapterJsonFile succeeds but probeAdapterHealth fails, state file persists with stale PID. cleanupFailedAdapterStartup only called on exception, not health probe false. No test covers this cleanup gap.
  • Impact: Stale state file causes subsequent onboarding to believe healthy adapter exists.
  • Recommended action: Add test: ensureOpenRouterRuntimeAdapterLocked cleans up state file when health probe fails post-write.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Mock health probe to fail after state write; verify removeLocalAdapterFile(STATE_PATH) called.
  • Missing regression test: Add test for state file cleanup on health probe failure after successful write.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Mock health probe to fail after state write; verify removeLocalAdapterFile(STATE_PATH) called.
  • Evidence: lifecycle.ts:218-230 writeLocalAdapterJsonFile followed by probeAdapterHealth lifecycle.ts:231-233 throw on health probe failure without cleanup lifecycle.ts:132-135 cleanupFailedAdapterStartup only in catch block Correctness finding already identified this behavior mismatch

PRA-20 Resolve/justify — Missing test: adapter restart with prior PID/state converges without ghost resources

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:294
  • Category: tests
  • Problem: lifecycle-state invariant requires 'partial failure and retry converge without ghost resources or stale ports'. No test exercises adapter restart scenario with existing PID/state files.
  • Impact: Ghost adapter processes or stale ports may persist across retries.
  • Recommended action: Add test: start adapter, kill process leaving PID/state, restart adapter -> prior state cleaned up, new adapter healthy.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run ensureOpenRouterRuntimeAdapter twice with simulated crash between; verify no ghost process and new adapter healthy.
  • Missing regression test: Add test for adapter restart convergence without ghost resources.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run ensureOpenRouterRuntimeAdapter twice with simulated crash between; verify no ghost process and new adapter healthy.
  • Evidence: riskPlan lifecycle-state invariant: 'partial failure and retry converge without ghost resources or stale ports' No test for adapter restart with prior state

PRA-21 Resolve/justify — Missing test: network policy permits sandbox -> host.openshell.internal:11437 and denies direct openrouter.ai

  • Location: src/lib/inference/openrouter-runtime-adapter-forward.ts:1
  • Category: tests
  • Problem: inference-policy invariant requires 'network policy permits the intended route and denies unintended egress'. No unit or mocked test verifies sandbox egress policy for adapter route vs direct OpenRouter.
  • Impact: Sandbox may have unintended direct egress to openrouter.ai or be blocked from adapter route.
  • Recommended action: Add mocked test verifying network policy allows host.openshell.internal:11437 and denies openrouter.ai egress.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check policy YAML for sandbox egress rules; verify adapter route permitted, direct OpenRouter denied.
  • Missing regression test: Add test for network policy egress rules specific to OpenRouter adapter route.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check policy YAML for sandbox egress rules; verify adapter route permitted, direct OpenRouter denied.
  • Evidence: riskPlan inference-policy invariant: 'network policy permits the intended route and denies unintended egress' No test for OpenRouter adapter network policy

💡 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 — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `ci/platform-matrix.json`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state (onboard-resume, onboard-repair), inference-policy (inference-routing, network-policy), platform-install (cloud-onboard). Unit/mocked tests cover adapter auth, forwarding, port validation, cleanup, catalog fallback but do not exercise real host-sandbox boundary, network policy egress, or adapter restart convergence.
  • PRA-T2 Runtime validation — Run the cloud-onboard E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: ci/platform-matrix.json.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state (onboard-resume, onboard-repair), inference-policy (inference-routing, network-policy), platform-install (cloud-onboard). Unit/mocked tests cover adapter auth, forwarding, port validation, cleanup, catalog fallback but do not exercise real host-sandbox boundary, network policy egress, or adapter restart convergence.
  • PRA-T3 Runtime validation — Run the `inference-routing` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/inference/openrouter-runtime-adapter-common.ts`, `src/lib/inference/openrouter-runtime-adapter-entry.ts`, `src/lib/inference/openrouter-runtime-adapter-forward.ts`, `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts`, `src/lib/inference/openrouter-runtime-adapter-server.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state (onboard-resume, onboard-repair), inference-policy (inference-routing, network-policy), platform-install (cloud-onboard). Unit/mocked tests cover adapter auth, forwarding, port validation, cleanup, catalog fallback but do not exercise real host-sandbox boundary, network policy egress, or adapter restart convergence.
  • PRA-T4 Runtime validation — Run the inference-routing E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: src/lib/inference/openrouter-runtime-adapter-common.ts, src/lib/inference/openrouter-runtime-adapter-entry.ts, src/lib/inference/openrouter-runtime-adapter-forward.ts, src/lib/inference/openrouter-runtime-adapter-lifecycle.ts, src/lib/inference/openrouter-runtime-adapter-server.ts.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state (onboard-resume, onboard-repair), inference-policy (inference-routing, network-policy), platform-install (cloud-onboard). Unit/mocked tests cover adapter auth, forwarding, port validation, cleanup, catalog fallback but do not exercise real host-sandbox boundary, network policy egress, or adapter restart convergence.
  • PRA-T5 Runtime validation — Run the `network-policy` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/inference/openrouter-runtime-adapter-common.ts`, `src/lib/inference/openrouter-runtime-adapter-entry.ts`, `src/lib/inference/openrouter-runtime-adapter-forward.ts`, `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts`, `src/lib/inference/openrouter-runtime-adapter-server.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state (onboard-resume, onboard-repair), inference-policy (inference-routing, network-policy), platform-install (cloud-onboard). Unit/mocked tests cover adapter auth, forwarding, port validation, cleanup, catalog fallback but do not exercise real host-sandbox boundary, network policy egress, or adapter restart convergence.
  • PRA-T6 Missing test: removeStaleLock non-ENOENT errors logged and do not block startup — Add test: simulate lock file statSync failure with EACCES, verify adapter startup proceeds and error is logged.
  • PRA-T7 Missing test: lock cleanup failure logged and does not permanently block startup — Add test: simulate lock file that cannot be unlinked, verify cleanup failure logged and subsequent startup attempts eventually succeed.
  • PRA-T8 Missing test: probeAdapterHealth classifies failure types (timeout vs address error) — Add test: probeAdapterHealth against invalid bind address returns false with logged address error; against refused connection returns false with logged connection error.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Monolith growth in uninstall run-plan test file

  • Location: src/lib/actions/uninstall/run-plan.test.ts:1437
  • Category: architecture
  • Problem: Test file grew by 8 lines (1437→1445) adding OpenRouter adapter cleanup test setup inline. Continued provider-specific test fixture accumulation increases maintenance burden.
  • Impact: Test file becomes harder to maintain and navigate as provider-specific cleanup logic accumulates linearly.
  • Recommended action: Extract OpenRouter adapter cleanup test setup into a dedicated test helper module (e.g., test/support/openrouter-adapter-test-setup.ts) and import it here.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line count of src/lib/actions/uninstall/run-plan.test.ts and whether OpenRouter adapter test setup is inlined or extracted.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line count of src/lib/actions/uninstall/run-plan.test.ts and whether OpenRouter adapter test setup is inlined or extracted.
  • Evidence: Monolith delta: baseLines=1437, headLines=1445, delta=8, severity=warning Test adds openrouter-runtime-adapter.pid/json/lock/log file setup in setupStateDir()

PRA-2 Resolve/justify — Monolith growth in uninstall run-plan implementation

  • Location: src/lib/actions/uninstall/run-plan.ts:992
  • Category: architecture
  • Problem: Implementation file grew by 2 lines (992→994) adding import and call to stopOpenRouterRuntimeAdapter. Continued provider-specific cleanup logic accumulation increases cognitive load.
  • Impact: Uninstall plan execution accumulates provider-specific steps inline, making the function harder to audit for completeness and ordering.
  • Recommended action: Consider a registry pattern where adapter cleanup functions self-register, or a dedicated cleanup orchestration module that collects all adapter stoppers.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/actions/uninstall/run-plan.ts for inline adapter cleanup calls vs. a registry/orchestrator.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/actions/uninstall/run-plan.ts for inline adapter cleanup calls vs. a registry/orchestrator.
  • Evidence: Monolith delta: baseLines=992, headLines=994, delta=2, severity=warning Added import and call to stopOpenRouterRuntimeAdapter in executePlan()

PRA-3 Resolve/justify — Monolith growth in setup-inference orchestration

  • Location: src/lib/onboard/setup-inference.ts:450
  • Category: architecture
  • Problem: Orchestration file grew by 3 lines (450→453) adding OpenRouter provider handling. Continued provider-specific onboarding logic accumulation increases cognitive load.
  • Impact: Setup-inference accumulates provider-specific branching, making it harder to audit for completeness and maintain.
  • Recommended action: Extract provider-specific onboarding into a strategy/registry pattern (e.g., InferenceProviderSetupStrategy interface) so each provider encapsulates its adapter setup, validation, and registration.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/onboard/setup-inference.ts for provider-specific branching vs. a strategy/registry pattern.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/onboard/setup-inference.ts for provider-specific branching vs. a strategy/registry pattern.
  • Evidence: Monolith delta: baseLines=450, headLines=453, delta=3, severity=warning Added OpenRouter provider handling in setup-inference flow

PRA-4 Resolve/justify — Duplicate port resolution logic; should import from ports.ts

  • Location: src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:29
  • Category: architecture
  • Problem: cleanup.ts reimplements port parsing and validation logic (resolveOpenRouterRuntimeAdapterPort) that already exists as OPENROUTER_RUNTIME_ADAPTER_PORT constant and validation in ports.ts.
  • Impact: Drift risk: port default/validation changes in ports.ts won't propagate to cleanup logic, causing inconsistency.
  • Recommended action: Import OPENROUTER_RUNTIME_ADAPTER_PORT from ../core/ports and use it directly. Remove resolveOpenRouterRuntimeAdapterPort and DEFAULT_OPENROUTER_RUNTIME_ADAPTER_PORT.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts for resolveOpenRouterRuntimeAdapterPort vs. import from ports.ts.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts for resolveOpenRouterRuntimeAdapterPort vs. import from ports.ts.
  • Evidence: cleanup.ts:29-42 defines resolveOpenRouterRuntimeAdapterPort with same logic as ports.ts ports.ts exports OPENROUTER_RUNTIME_ADAPTER_PORT constant

PRA-5 Resolve/justify — validateOpenRouterRuntimeAdapterPort duplicates gateway port validation logic

  • Location: src/lib/core/ports.ts:140
  • Category: architecture
  • Problem: New validateOpenRouterRuntimeAdapterPort function replicates conflict-checking logic from validateGatewayPort instead of sharing a common validator.
  • Impact: Drift risk: port conflict rules updated in one validator but not the other, leading to inconsistent validation.
  • Recommended action: Extract shared port conflict validation into a common internal function used by both validateGatewayPort and validateOpenRouterRuntimeAdapterPort.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare validateGatewayPort and validateOpenRouterRuntimeAdapterPort conflict-checking logic in ports.ts.
  • Missing regression test: N/A — architecture finding; no regression test needed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare validateGatewayPort and validateOpenRouterRuntimeAdapterPort conflict-checking logic in ports.ts.
  • Evidence: ports.ts:140-185 validateOpenRouterRuntimeAdapterPort replicates reservedDefaults and conflicts arrays from validateGatewayPort Both functions check dashboard range, reserved defaults, and configured port conflicts

PRA-6 Required — Authorization hash persisted to disk violates 'nothing written to disk' acceptance and enables offline attacks

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225
  • Category: security
  • Problem: Issue feat: adding OpenRouter as an inference provider #5826 explicitly requires 'nothing is written to disk' for the OpenRouter API key. The adapter writes a SHA-256 hash of the key (authorizationHash) to ~/.nemoclaw/openrouter-runtime-adapter.json with 0o600 permissions. While the raw key is not stored, the persisted hash enables offline verification attacks against guessed API keys and remains on disk after onboarding.
  • Impact: Credential-derived secret persisted contrary to acceptance criteria; offline brute-force verification of API keys possible; file persists after onboarding.
  • Required action: Either (a) update issue feat: adding OpenRouter as an inference provider #5826 to acknowledge hash persistence as deliberate tradeoff for adapter restart survival, or (b) redesign to keep hash only in memory (require re-auth on adapter restart). Option (a) with explicit issue update is pragmatic given PR body documents this boundary.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run onboarding with OpenRouter, inspect ~/.nemoclaw/openrouter-runtime-adapter.json for authorizationHash field (64-char hex).
  • Missing regression test: Add test verifying adapter state file contains authorizationHash after onboarding; document as expected behavior per updated acceptance.
  • Done when: The required change is committed and verification passes: Run onboarding with OpenRouter, inspect ~/.nemoclaw/openrouter-runtime-adapter.json for authorizationHash field (64-char hex).
  • Evidence: lifecycle.ts:225-230 writeLocalAdapterJsonFile includes authorizationHash common.ts:77-80 adapterAuthorizationHash creates SHA-256 of token Issue feat: adding OpenRouter as an inference provider #5826 body: 'nothing is written to disk' inference-options.mdx:96 claims 'adapter does not store the API key' but omits hash

PRA-7 Resolve/justify — Adapter binds 0.0.0.0 instead of loopback, expanding attack surface

  • Location: src/lib/inference/openrouter.ts:18
  • Category: security
  • Problem: OPENROUTER_RUNTIME_ADAPTER_BIND_HOST is set to '0.0.0.0', causing the adapter to listen on all interfaces. Since sandbox traffic reaches the adapter via host.openshell.internal (which resolves to host loopback), binding 0.0.0.0 is unnecessary and expands the attack surface if an auth bypass is discovered.
  • Impact: Adapter accessible on all host interfaces instead of only loopback; increased attack surface for auth bypass or DoS.
  • Recommended action: Change OPENROUTER_RUNTIME_ADAPTER_BIND_HOST to '127.0.0.1' since host.openshell.internal resolves to loopback. If container networking requires 0.0.0.0, document the justification and add network policy to restrict access.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check openrouter.ts:18 for OPENROUTER_RUNTIME_ADAPTER_BIND_HOST value; verify host.openshell.internal resolves to 127.0.0.1 in sandbox.
  • Missing regression test: Add test that adapter binds only 127.0.0.1 (or document why 0.0.0.0 is required with network policy).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check openrouter.ts:18 for OPENROUTER_RUNTIME_ADAPTER_BIND_HOST value; verify host.openshell.internal resolves to 127.0.0.1 in sandbox.
  • Evidence: openrouter.ts:18 OPENROUTER_RUNTIME_ADAPTER_BIND_HOST = '0.0.0.0' openrouter.ts:21 OPENROUTER_RUNTIME_ADAPTER_LOOPBACK_HOST = '127.0.0.1' Sandbox route uses host.openshell.internal:11437

PRA-8 Resolve/justify — Multiple silent error handling paths lose observability for security events

  • Location: src/lib/inference/openrouter-runtime-adapter-common.ts:48
  • Category: security
  • Problem: defaultAdapterLogger and logAdapterEvent swallow log write failures (console.error only). removeStaleLock release fn swallows unlinkSync errors. probeAdapterHealth catches all errors -> false (masks bind/route/config errors). entry.ts top-level catch -> exit 1 (no failure classification). cleanup PID read swallows errors. These silent failures reduce visibility into adapter health and potential security events.
  • Impact: Operators cannot detect logging failures, lock cleanup failures, or distinguish health probe failure types; security-relevant events may go unnoticed.
  • Recommended action: Propagate log write errors via metric/side-channel. Log lock cleanup failures. Classify health probe failures (timeout/connection refused vs address/route errors). Classify startup failures with distinct exit codes. Log PID file read failures before falling back to lsof scan.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Simulate each silent failure (readonly fs for logs, immutable lock file, invalid bind address, missing env, corrupt PID file) and verify failures are logged/classified.
  • Missing regression test: Add tests for each silent failure path verifying structured logging/classification.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Simulate each silent failure (readonly fs for logs, immutable lock file, invalid bind address, missing env, corrupt PID file) and verify failures are logged/classified.
  • Evidence: common.ts:44-50 defaultAdapterLogger try/catch -> console.error only common.ts:55-63 logAdapterEvent try/catch -> console.error only lifecycle.ts:92-95 release fn bare catch lifecycle.ts:149-178 probeAdapterHealth catches all -> false entry.ts:3-9 try/catch -> exit 1 cleanup.ts:114-118 try/catch swallows PID read errors

PRA-9 Resolve/justify — Adapter port validation missing Hermes port 8642 conflict check

  • Location: src/lib/core/ports.ts:156
  • Category: security
  • Problem: validateOpenRouterRuntimeAdapterPort checks conflicts against gateway, dashboard range, vLLM (8000), Ollama (11434), Ollama proxy (11435), Bedrock adapter (11436), but omits HERMES_OPENAI_API_PORT (8642). validateGatewayPort includes Hermes port in reservedDefaults. OpenRouter adapter port could be set to 8642, conflicting with Hermes OpenAI-compatible API.
  • Impact: Port conflict between OpenRouter runtime adapter and Hermes API port, breaking Hermes onboarding or adapter startup.
  • Recommended action: Add { label: 'Hermes OpenAI-compatible API', port: 8642 } to reservedDefaults in validateOpenRouterRuntimeAdapterPort.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Set NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT=8642 and run port validation; should reject with Hermes conflict.
  • Missing regression test: Add test that validateOpenRouterRuntimeAdapterPort rejects 8642 with Hermes label.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Set NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT=8642 and run port validation; should reject with Hermes conflict.
  • Evidence: ports.ts:91 validateGatewayPort reservedDefaults includes Hermes port 8642 ports.ts:156-163 validateOpenRouterRuntimeAdapterPort reservedDefaults omits Hermes port

PRA-10 Resolve/justify — Runtime adapter workaround for missing OpenShell L7 middleware lacks source fix plan

  • Location: src/lib/onboard/openrouter-runtime.ts:10
  • Category: security
  • Problem: TODO comment acknowledges adapter is a workaround pending OpenShell L7 middleware/default-header injection, but no upstream issue link, regression test proving adapter still needed, or removal timeline. Workaround persists hash to disk (violating acceptance), binds 0.0.0.0 unnecessarily, and adds a new trusted-code component. Without a plan to remove it, this becomes permanent technical debt in the trust boundary.
  • Impact: Workaround may become permanent; no verification that OpenShell cannot be fixed; adapter adds attack surface and trust boundary complexity.
  • Recommended action: Link to upstream OpenShell issue, add regression test that fails when OpenShell gains L7 middleware/header injection (proving adapter removable), and document removal criteria.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check openrouter-runtime.ts:10 for TODO with upstream issue link, regression test, and removal criteria.
  • Missing regression test: Add test that fails when OpenShell gains L7 middleware/header injection capability, proving adapter can be removed.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check openrouter-runtime.ts:10 for TODO with upstream issue link, regression test, and removal criteria.
  • Evidence: openrouter-runtime.ts:10-14 TODO documents workaround No upstream issue reference, regression test, or removal timeline Adapter persists hash to disk, binds 0.0.0.0, adds new trusted component

PRA-11 Resolve/justify — Missing test: removeStaleLock non-ENOENT errors logged and do not block startup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:79
  • Category: tests
  • Problem: removeStaleLock throws on non-ENOENT filesystem errors (e.g., EACCES). tryAcquireAdapterLock calls it without try/catch, so adapter startup can fail catastrophically instead of degrading gracefully. No test covers this error path.
  • Impact: Adapter startup may fail permanently on lock directory filesystem issues without test coverage proving graceful degradation.
  • Recommended action: Add test: simulate lock file statSync failure with EACCES, verify adapter startup proceeds and error is logged.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run test with mocked fs.statSync throwing EACCES; verify tryAcquireAdapterLock does not throw and logs error.
  • Missing regression test: Add test covering removeStaleLock non-ENOENT error handling in tryAcquireAdapterLock.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run test with mocked fs.statSync throwing EACCES; verify tryAcquireAdapterLock does not throw and logs error.
  • Evidence: lifecycle.ts:74-80 removeStaleLock throws on non-ENOENT lifecycle.ts:87 calls removeStaleLock without try/catch No existing test for this error path

PRA-12 Resolve/justify — Missing test: lock cleanup failure logged and does not permanently block startup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:95
  • Category: tests
  • Problem: Lock release function uses bare catch block (best-effort cleanup). If fs.unlinkSync fails (immutable file, permission change), lock persists indefinitely. No test verifies failure is logged and startup eventually succeeds.
  • Impact: Stale lock may permanently block adapter startup without detection.
  • Recommended action: Add test: simulate lock file that cannot be unlinked, verify cleanup failure logged and subsequent startup attempts eventually succeed.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Create immutable lock file, run adapter startup, verify warning logged and startup succeeds after lock TTL or manual removal.
  • Missing regression test: Add test for lock cleanup failure observability and non-permanent blocking.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Create immutable lock file, run adapter startup, verify warning logged and startup succeeds after lock TTL or manual removal.
  • Evidence: lifecycle.ts:92-95 release fn bare catch swallows all errors Comment acknowledges best-effort only No test for lock cleanup failure path

PRA-13 Resolve/justify — Missing test: probeAdapterHealth classifies failure types (timeout vs address error)

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:173
  • Category: tests
  • Problem: probeAdapterHealth catches all errors and returns false. No test verifies distinction between connection refused (adapter down) and address/route errors (config issue).
  • Impact: Operators cannot distinguish transient vs configuration failures from health probe results.
  • Recommended action: Add test: probeAdapterHealth against invalid bind address returns false with logged address error; against refused connection returns false with logged connection error.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run probeAdapterHealth with invalid hostname and with nothing listening; verify distinct log entries.
  • Missing regression test: Add test classifying health probe failure types with distinct logging.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run probeAdapterHealth with invalid hostname and with nothing listening; verify distinct log entries.
  • Evidence: lifecycle.ts:149-178 probeAdapterHealth catches all -> resolve(false) No error classification or logging in probe path No test for failure classification

PRA-14 Resolve/justify — Missing tests: startup failure classification with distinct exit codes

  • Location: src/lib/inference/openrouter-runtime-adapter-entry.ts:8
  • Category: tests
  • Problem: Entry point exits with code 1 for all failures. No tests for classified failures: port conflict (2), missing env (3), lock contention (4), upstream config (5).
  • Impact: Automation cannot programmatically distinguish startup failure modes.
  • Recommended action: Add tests for each classified startup failure mode with expected exit codes.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Simulate each failure mode and verify distinct exit codes.
  • Missing regression test: Add tests for exit codes 2 (port conflict), 3 (missing env), 4 (lock contention), 5 (upstream config).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Simulate each failure mode and verify distinct exit codes.
  • Evidence: entry.ts:3-9 try/catch -> process.exit(1) for all errors No failure classification logic No tests for classified exit codes

PRA-15 Resolve/justify — Missing test: log write failures reported via metric/side-channel

  • Location: src/lib/inference/openrouter-runtime-adapter-common.ts:48
  • Category: tests
  • Problem: defaultAdapterLogger and logAdapterEvent swallow log write failures (console.error only). No test verifies failures are reported beyond console.error.
  • Impact: Logging failures (disk full, permissions) go undetected by monitoring.
  • Recommended action: Add test: simulate log write failure (readonly fs), verify failure reported via metric or side-channel.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Mock appendLocalAdapterJsonLine to throw, call defaultAdapterLogger, verify side-channel metric incremented.
  • Missing regression test: Add test for log write failure observability beyond console.error.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Mock appendLocalAdapterJsonLine to throw, call defaultAdapterLogger, verify side-channel metric incremented.
  • Evidence: common.ts:44-50 defaultAdapterLogger try/catch -> console.error only common.ts:55-63 logAdapterEvent try/catch -> console.error only No side-channel/metric reporting

PRA-16 Resolve/justify — Missing test: corrupt PID file + missing lsof still attempts cleanup

  • Location: src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts:118
  • Category: tests
  • Problem: PID file read errors silently ignored; fallback depends on lsof availability. No test covers corrupt PID file with lsof unavailable.
  • Impact: Orphan adapter process may persist if PID file unreadable and lsof unavailable.
  • Recommended action: Add test: corrupt PID file + missing lsof -> warning logged and alternative cleanup attempted.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Create corrupt PID file, mock commandExists('lsof') false, run cleanup, verify warning and fallback attempt.
  • Missing regression test: Add test for PID read failure + lsof unavailable cleanup path.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Create corrupt PID file, mock commandExists('lsof') false, run cleanup, verify warning and fallback attempt.
  • Evidence: cleanup.ts:114-118 try/catch swallows PID read errors cleanup.ts:122-125 lsof check with warn if missing No test for this combined failure scenario

PRA-17 Resolve/justify — Missing test: validateOpenRouterRuntimeAdapterPort rejects Hermes port 8642

  • Location: src/lib/core/ports.test.ts:162
  • Category: tests
  • Problem: Port validation for OpenRouter adapter omits Hermes port 8642 check (present in gateway validation). No test covers this conflict.
  • Impact: Port conflict between OpenRouter adapter and Hermes API possible without test detection.
  • Recommended action: Add test: validateOpenRouterRuntimeAdapterPort rejects 8642 with Hermes label.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run ports.test.ts with validateOpenRouterRuntimeAdapterPort(8642); expect throw with Hermes label.
  • Missing regression test: Add test case in ports.test.ts for Hermes port 8642 rejection.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run ports.test.ts with validateOpenRouterRuntimeAdapterPort(8642); expect throw with Hermes label.
  • Evidence: ports.ts:156-163 validateOpenRouterRuntimeAdapterPort reservedDefaults omits Hermes port ports.test.ts has no test for 8642 conflict PRA-12 and security finding F-010 identify this gap

PRA-18 Resolve/justify — Missing regression test: adapter removable when OpenShell gains L7 middleware

  • Location: src/lib/onboard/openrouter-runtime.ts:10
  • Category: tests
  • Problem: TODO acknowledges adapter is workaround pending OpenShell L7 middleware. No test fails when OpenShell gains this capability, proving adapter can be removed.
  • Impact: Workaround may become permanent technical debt without detection.
  • Recommended action: Add test that fails (or detects capability) when OpenShell exposes L7 middleware/default-header injection for provider routes.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check for OpenShell L7 middleware capability in test; fail if present (proving adapter removable).
  • Missing regression test: Add test detecting OpenShell L7 middleware capability as removal signal for adapter.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check for OpenShell L7 middleware capability in test; fail if present (proving adapter removable).
  • Evidence: openrouter-runtime.ts:10-14 TODO documents workaround No upstream issue, regression test, or removal criteria F-006 and F-011 require this test

PRA-19 Resolve/justify — Missing test: state file cleaned up when health probe fails after write

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:225
  • Category: tests
  • Problem: If writeLocalAdapterJsonFile succeeds but probeAdapterHealth fails, state file persists with stale PID. cleanupFailedAdapterStartup only called on exception, not health probe false. No test covers this cleanup gap.
  • Impact: Stale state file causes subsequent onboarding to believe healthy adapter exists.
  • Recommended action: Add test: ensureOpenRouterRuntimeAdapterLocked cleans up state file when health probe fails post-write.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Mock health probe to fail after state write; verify removeLocalAdapterFile(STATE_PATH) called.
  • Missing regression test: Add test for state file cleanup on health probe failure after successful write.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Mock health probe to fail after state write; verify removeLocalAdapterFile(STATE_PATH) called.
  • Evidence: lifecycle.ts:218-230 writeLocalAdapterJsonFile followed by probeAdapterHealth lifecycle.ts:231-233 throw on health probe failure without cleanup lifecycle.ts:132-135 cleanupFailedAdapterStartup only in catch block Correctness finding already identified this behavior mismatch

PRA-20 Resolve/justify — Missing test: adapter restart with prior PID/state converges without ghost resources

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:294
  • Category: tests
  • Problem: lifecycle-state invariant requires 'partial failure and retry converge without ghost resources or stale ports'. No test exercises adapter restart scenario with existing PID/state files.
  • Impact: Ghost adapter processes or stale ports may persist across retries.
  • Recommended action: Add test: start adapter, kill process leaving PID/state, restart adapter -> prior state cleaned up, new adapter healthy.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run ensureOpenRouterRuntimeAdapter twice with simulated crash between; verify no ghost process and new adapter healthy.
  • Missing regression test: Add test for adapter restart convergence without ghost resources.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run ensureOpenRouterRuntimeAdapter twice with simulated crash between; verify no ghost process and new adapter healthy.
  • Evidence: riskPlan lifecycle-state invariant: 'partial failure and retry converge without ghost resources or stale ports' No test for adapter restart with prior state

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 Jul 9, 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: Clean up a newly started OpenRouter adapter when onboarding fails after startup.
Open items: 0 required · 4 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 3 still apply · 1 new item found

Action checklist

  • PRA-1 Resolve or justify: Clean up a newly started OpenRouter adapter when onboarding fails after startup in src/lib/onboard/openrouter-runtime.ts:86
  • PRA-2 Resolve or justify: Bind the OpenRouter runtime adapter to the narrowest required interface in src/lib/inference/openrouter.ts:18
  • PRA-3 Resolve or justify: Do not expose the OpenRouter bearer-token verifier on unauthenticated health in src/lib/inference/openrouter-runtime-adapter-server.ts:76
  • PRA-4 Resolve or justify: Add direct lifecycle tests for OpenRouter adapter startup cleanup in src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274
  • 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: Add direct lifecycle tests for OpenRouter adapter startup cleanup
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: OpenRouter onboarding downstream cleanup after adapter startup

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify correctness src/lib/onboard/openrouter-runtime.ts:86 Thread a started/reused signal from `ensureOpenRouterRuntimeAdapter` (or add an equivalent ownership token) and wrap the downstream registration, inference-set, route/smoke validation, and registry-update steps in cleanup-on-failure for adapters started by this call; preserve reused adapters on the same failure path.
PRA-2 Resolve/justify security src/lib/inference/openrouter.ts:18 Bind to the narrowest address/interface that still supports the OpenShell `host.openshell.internal` route, or require an explicit opt-in/all-interface setting with a warning and a checked interface/firewall constraint. Add regression coverage for the default bind target.
PRA-3 Resolve/justify security src/lib/inference/openrouter-runtime-adapter-server.ts:76 Remove `authorizationHash` from the unauthenticated health response, or require the same bearer authorization for sensitive health details while keeping only non-sensitive readiness data public. Update lifecycle probing to use an internal/authenticated check if it needs to verify the hash.
PRA-4 Resolve/justify tests src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274 Add direct lifecycle unit tests around `ensureOpenRouterRuntimeAdapter` by injecting or stubbing process, file, and health dependencies so failures after PID persistence and after state write can be simulated and cleanup assertions made.
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 — Clean up a newly started OpenRouter adapter when onboarding fails after startup

  • Location: src/lib/onboard/openrouter-runtime.ts:86
  • Category: correctness
  • Problem: `setupOpenRouterRuntimeInference` starts or reuses the OpenRouter runtime adapter and then can fail during provider registration, `inference set`, route verification, smoke validation, or registry update without stopping an adapter that was newly started for this setup attempt.
  • Impact: A failed OpenRouter onboarding can leave a ghost adapter process plus PID/state files on the host, so retries may reuse stale state or require uninstall cleanup instead of converging from a clean partial-failure state.
  • Recommended action: Thread a started/reused signal from `ensureOpenRouterRuntimeAdapter` (or add an equivalent ownership token) and wrap the downstream registration, inference-set, route/smoke validation, and registry-update steps in cleanup-on-failure for adapters started by this call; preserve reused adapters on the same failure path.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/openrouter-runtime.ts:72-141` and confirm that errors after `ensureAdapter` returns have no cleanup call before returning or throwing.
  • Missing regression test: Add an onboarding test where `ensureAdapter` reports a newly started adapter and `upsertProvider`, `runOpenshell`, `verifyInferenceRoute`, `verifyOnboardInferenceSmoke`, and/or `updateSandbox` fails; assert cleanup is called for a fresh adapter and not called for a reused adapter.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/openrouter-runtime.ts:72-141` and confirm that errors after `ensureAdapter` returns have no cleanup call before returning or throwing.
  • Evidence: src/lib/onboard/openrouter-runtime.ts:72 starts or reuses the adapter before downstream setup work. src/lib/onboard/openrouter-runtime.ts:86-122 returns or exits on provider registration and `inference set` failures without stopping the adapter. src/lib/onboard/openrouter-runtime.ts:125-141 calls route verification, smoke validation, registry update, and success logging without a cleanup-on-error wrapper. The risk-plan lifecycle-state invariant requires partial failure and retry to converge without ghost resources or stale ports.

PRA-2 Resolve/justify — Bind the OpenRouter runtime adapter to the narrowest required interface

  • Location: src/lib/inference/openrouter.ts:18
  • Category: security
  • Problem: The new OpenRouter runtime adapter binds its HTTP server to `0.0.0.0` by default, exposing a credential-bearing inference adapter service on every host interface instead of the narrow host/container route that needs it.
  • Impact: A same-network or host-adjacent attacker can reach the adapter surface, probe unauthenticated health metadata, and attempt misuse if the OpenRouter bearer value is leaked elsewhere. This increases attack surface for a host inference credential boundary beyond least exposure.
  • Recommended action: Bind to the narrowest address/interface that still supports the OpenShell `host.openshell.internal` route, or require an explicit opt-in/all-interface setting with a warning and a checked interface/firewall constraint. Add regression coverage for the default bind target.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/inference/openrouter.ts:18` and `src/lib/inference/openrouter-runtime-adapter-server.ts:161-170` to confirm the default bind host is `0.0.0.0` and is passed directly to `server.listen`.
  • Missing regression test: Add a config/server startup test asserting the default bind target is loopback/bridge-scoped rather than all interfaces, or asserting all-interface binding only happens behind an explicit opt-in and warning.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/inference/openrouter.ts:18` and `src/lib/inference/openrouter-runtime-adapter-server.ts:161-170` to confirm the default bind host is `0.0.0.0` and is passed directly to `server.listen`.
  • Evidence: src/lib/inference/openrouter.ts:18 defines `OPENROUTER_RUNTIME_ADAPTER_BIND_HOST = "0.0.0.0"`. src/lib/inference/openrouter-runtime-adapter-server.ts:161-170 passes `OPENROUTER_RUNTIME_ADAPTER_BIND_HOST` to `server.listen` and logs that listener. The adapter handles a credential-bearing inference boundary by requiring exact bearer matching before forwarding runtime requests.

PRA-3 Resolve/justify — Do not expose the OpenRouter bearer-token verifier on unauthenticated health

  • Location: src/lib/inference/openrouter-runtime-adapter-server.ts:76
  • Category: security
  • Problem: The adapter's unauthenticated `/health` endpoint returns `authorizationHash`, the SHA-256 verifier for the OpenRouter bearer token accepted by runtime paths.
  • Impact: Any host or network peer that can reach the adapter can collect a stable fingerprint/verifier for the OpenRouter credential. While it is not the raw API key, it is unnecessary sensitive metadata and compounds the risk of the all-interface listener.
  • Recommended action: Remove `authorizationHash` from the unauthenticated health response, or require the same bearer authorization for sensitive health details while keeping only non-sensitive readiness data public. Update lifecycle probing to use an internal/authenticated check if it needs to verify the hash.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/inference/openrouter-runtime-adapter-server.ts:76-83` and `src/lib/inference/openrouter-runtime-adapter.test.ts:130-139` to confirm `/health` returns `authorizationHash` without auth and the test expects it.
  • Missing regression test: Add an adapter health test that unauthenticated `/health` does not include `authorizationHash` or other credential verifier material, plus an authenticated/internal health test if lifecycle still verifies the bound authorization hash.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/inference/openrouter-runtime-adapter-server.ts:76-83` and `src/lib/inference/openrouter-runtime-adapter.test.ts:130-139` to confirm `/health` returns `authorizationHash` without auth and the test expects it.
  • Evidence: src/lib/inference/openrouter-runtime-adapter-server.ts:76-83 sends `authorizationHash` in the `/health` JSON response before any authorization check. src/lib/inference/openrouter-runtime-adapter.test.ts:130-139 fetches `/health` without auth and asserts `authorizationHash` is present while only checking the raw token is absent. src/lib/inference/openrouter-runtime-adapter-server.ts:86-101 gates runtime forwarding paths after the unauthenticated health branch.

PRA-4 Resolve/justify — Add direct lifecycle tests for OpenRouter adapter startup cleanup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274
  • Category: tests
  • Problem: The new OpenRouter adapter lifecycle persists a detached child PID, waits for health, writes state, re-probes health, and removes PID/state on startup failure, but no checked-in test exercises `ensureOpenRouterRuntimeAdapter` or the `cleanupFailedAdapterStartup` path.
  • Impact: A regression in startup failure cleanup could leave stale PID/state files or a ghost adapter process that later retries reuse incorrectly, breaking the deterministic lifecycle-state invariant that partial failures converge without ghost resources or stale ports.
  • Recommended action: Add direct lifecycle unit tests around `ensureOpenRouterRuntimeAdapter` by injecting or stubbing process, file, and health dependencies so failures after PID persistence and after state write can be simulated and cleanup assertions made.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search checked-in tests for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, or `probeAdapterHealth`; no matches are present, while `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274-295` contains the persist/wait/write/reprobe/cleanup sequence.
  • Missing regression test: A unit test that stubs or injects `spawnDetachedNodeAdapter`, `persistLocalAdapterPid`, `waitForLocalAdapterHealth`/`probeAdapterHealth`, `killLocalAdapterPid`, and state-file removal to prove failures after PID persistence and after state write clean up PID/state and that a subsequent retry cannot reuse the failed PID/state.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search checked-in tests for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, or `probeAdapterHealth`; no matches are present, while `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274-295` contains the persist/wait/write/reprobe/cleanup sequence.
  • Evidence: Repository grep for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, and `probeAdapterHealth` in `**/*test*.ts` returned no matches. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274 persists the child PID before readiness is confirmed. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:282-289 writes adapter state and then re-probes health. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:293-295 calls `cleanupFailedAdapterStartup()` on errors, but that behavior is untested.

💡 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 — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `ci/platform-matrix.json`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Static coverage is substantial for adapter forwarding, auth gating, onboarding wiring, port conflicts, and uninstall cleanup, but the risk plan includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation. Missing checked-in lifecycle regression coverage is represented by F-004, and the other missing behavior-specific tests are attached to F-001/F-002/F-003.
  • PRA-T2 Runtime validation — Add an OpenRouter onboarding test where a freshly started adapter is followed by `upsertProvider`, `inference set`, route verification, smoke validation, or registry-update failure, and assert cleanup runs only for fresh adapters, not reused adapters.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Static coverage is substantial for adapter forwarding, auth gating, onboarding wiring, port conflicts, and uninstall cleanup, but the risk plan includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation. Missing checked-in lifecycle regression coverage is represented by F-004, and the other missing behavior-specific tests are attached to F-001/F-002/F-003.
  • PRA-T3 Runtime validation — Run the `inference-routing` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/inference/openrouter-runtime-adapter-common.ts`, `src/lib/inference/openrouter-runtime-adapter-entry.ts`, `src/lib/inference/openrouter-runtime-adapter-forward.ts`, `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts`, `src/lib/inference/openrouter-runtime-adapter-server.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Static coverage is substantial for adapter forwarding, auth gating, onboarding wiring, port conflicts, and uninstall cleanup, but the risk plan includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation. Missing checked-in lifecycle regression coverage is represented by F-004, and the other missing behavior-specific tests are attached to F-001/F-002/F-003.
  • PRA-T4 Runtime validation — Add a config/server startup test asserting the OpenRouter runtime adapter default bind target is loopback/bridge-scoped rather than `0.0.0.0`, or that all-interface binding requires explicit opt-in and warning.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Static coverage is substantial for adapter forwarding, auth gating, onboarding wiring, port conflicts, and uninstall cleanup, but the risk plan includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation. Missing checked-in lifecycle regression coverage is represented by F-004, and the other missing behavior-specific tests are attached to F-001/F-002/F-003.
  • PRA-T5 Runtime validation — Run the `network-policy` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/inference/openrouter-runtime-adapter-common.ts`, `src/lib/inference/openrouter-runtime-adapter-entry.ts`, `src/lib/inference/openrouter-runtime-adapter-forward.ts`, `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts`, `src/lib/inference/openrouter-runtime-adapter-server.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Static coverage is substantial for adapter forwarding, auth gating, onboarding wiring, port conflicts, and uninstall cleanup, but the risk plan includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation. Missing checked-in lifecycle regression coverage is represented by F-004, and the other missing behavior-specific tests are attached to F-001/F-002/F-003.
  • PRA-T6 Add direct lifecycle tests for OpenRouter adapter startup cleanup — Add direct lifecycle unit tests around `ensureOpenRouterRuntimeAdapter` by injecting or stubbing process, file, and health dependencies so failures after PID persistence and after state write can be simulated and cleanup assertions made.
  • PRA-T7 Acceptance clause@ericksoa @jyaunches was chatting with Kasikrit and Kris about this. This seems like the right direction for enterprise inference support and providing a path for non rate limited models. How big is this work? Is it something we can ship in Sprint 8 (next Sprint)? — add test evidence or identify existing coverage. This issue comment is planning/scheduling context rather than a verifiable code behavior; the implementation direction is represented by the OpenRouter provider and runtime adapter changes.
  • PRA-T8 OpenRouter onboarding downstream cleanup after adapter startup — Add onboarding tests that fail each downstream step and assert cleanup for fresh adapters and preservation for reused adapters.. F-001 covers this source-of-truth item with current code evidence from `src/lib/onboard/openrouter-runtime.ts:72-141`.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Clean up a newly started OpenRouter adapter when onboarding fails after startup

  • Location: src/lib/onboard/openrouter-runtime.ts:86
  • Category: correctness
  • Problem: `setupOpenRouterRuntimeInference` starts or reuses the OpenRouter runtime adapter and then can fail during provider registration, `inference set`, route verification, smoke validation, or registry update without stopping an adapter that was newly started for this setup attempt.
  • Impact: A failed OpenRouter onboarding can leave a ghost adapter process plus PID/state files on the host, so retries may reuse stale state or require uninstall cleanup instead of converging from a clean partial-failure state.
  • Recommended action: Thread a started/reused signal from `ensureOpenRouterRuntimeAdapter` (or add an equivalent ownership token) and wrap the downstream registration, inference-set, route/smoke validation, and registry-update steps in cleanup-on-failure for adapters started by this call; preserve reused adapters on the same failure path.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/openrouter-runtime.ts:72-141` and confirm that errors after `ensureAdapter` returns have no cleanup call before returning or throwing.
  • Missing regression test: Add an onboarding test where `ensureAdapter` reports a newly started adapter and `upsertProvider`, `runOpenshell`, `verifyInferenceRoute`, `verifyOnboardInferenceSmoke`, and/or `updateSandbox` fails; assert cleanup is called for a fresh adapter and not called for a reused adapter.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/openrouter-runtime.ts:72-141` and confirm that errors after `ensureAdapter` returns have no cleanup call before returning or throwing.
  • Evidence: src/lib/onboard/openrouter-runtime.ts:72 starts or reuses the adapter before downstream setup work. src/lib/onboard/openrouter-runtime.ts:86-122 returns or exits on provider registration and `inference set` failures without stopping the adapter. src/lib/onboard/openrouter-runtime.ts:125-141 calls route verification, smoke validation, registry update, and success logging without a cleanup-on-error wrapper. The risk-plan lifecycle-state invariant requires partial failure and retry to converge without ghost resources or stale ports.

PRA-2 Resolve/justify — Bind the OpenRouter runtime adapter to the narrowest required interface

  • Location: src/lib/inference/openrouter.ts:18
  • Category: security
  • Problem: The new OpenRouter runtime adapter binds its HTTP server to `0.0.0.0` by default, exposing a credential-bearing inference adapter service on every host interface instead of the narrow host/container route that needs it.
  • Impact: A same-network or host-adjacent attacker can reach the adapter surface, probe unauthenticated health metadata, and attempt misuse if the OpenRouter bearer value is leaked elsewhere. This increases attack surface for a host inference credential boundary beyond least exposure.
  • Recommended action: Bind to the narrowest address/interface that still supports the OpenShell `host.openshell.internal` route, or require an explicit opt-in/all-interface setting with a warning and a checked interface/firewall constraint. Add regression coverage for the default bind target.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/inference/openrouter.ts:18` and `src/lib/inference/openrouter-runtime-adapter-server.ts:161-170` to confirm the default bind host is `0.0.0.0` and is passed directly to `server.listen`.
  • Missing regression test: Add a config/server startup test asserting the default bind target is loopback/bridge-scoped rather than all interfaces, or asserting all-interface binding only happens behind an explicit opt-in and warning.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/inference/openrouter.ts:18` and `src/lib/inference/openrouter-runtime-adapter-server.ts:161-170` to confirm the default bind host is `0.0.0.0` and is passed directly to `server.listen`.
  • Evidence: src/lib/inference/openrouter.ts:18 defines `OPENROUTER_RUNTIME_ADAPTER_BIND_HOST = "0.0.0.0"`. src/lib/inference/openrouter-runtime-adapter-server.ts:161-170 passes `OPENROUTER_RUNTIME_ADAPTER_BIND_HOST` to `server.listen` and logs that listener. The adapter handles a credential-bearing inference boundary by requiring exact bearer matching before forwarding runtime requests.

PRA-3 Resolve/justify — Do not expose the OpenRouter bearer-token verifier on unauthenticated health

  • Location: src/lib/inference/openrouter-runtime-adapter-server.ts:76
  • Category: security
  • Problem: The adapter's unauthenticated `/health` endpoint returns `authorizationHash`, the SHA-256 verifier for the OpenRouter bearer token accepted by runtime paths.
  • Impact: Any host or network peer that can reach the adapter can collect a stable fingerprint/verifier for the OpenRouter credential. While it is not the raw API key, it is unnecessary sensitive metadata and compounds the risk of the all-interface listener.
  • Recommended action: Remove `authorizationHash` from the unauthenticated health response, or require the same bearer authorization for sensitive health details while keeping only non-sensitive readiness data public. Update lifecycle probing to use an internal/authenticated check if it needs to verify the hash.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/inference/openrouter-runtime-adapter-server.ts:76-83` and `src/lib/inference/openrouter-runtime-adapter.test.ts:130-139` to confirm `/health` returns `authorizationHash` without auth and the test expects it.
  • Missing regression test: Add an adapter health test that unauthenticated `/health` does not include `authorizationHash` or other credential verifier material, plus an authenticated/internal health test if lifecycle still verifies the bound authorization hash.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/inference/openrouter-runtime-adapter-server.ts:76-83` and `src/lib/inference/openrouter-runtime-adapter.test.ts:130-139` to confirm `/health` returns `authorizationHash` without auth and the test expects it.
  • Evidence: src/lib/inference/openrouter-runtime-adapter-server.ts:76-83 sends `authorizationHash` in the `/health` JSON response before any authorization check. src/lib/inference/openrouter-runtime-adapter.test.ts:130-139 fetches `/health` without auth and asserts `authorizationHash` is present while only checking the raw token is absent. src/lib/inference/openrouter-runtime-adapter-server.ts:86-101 gates runtime forwarding paths after the unauthenticated health branch.

PRA-4 Resolve/justify — Add direct lifecycle tests for OpenRouter adapter startup cleanup

  • Location: src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274
  • Category: tests
  • Problem: The new OpenRouter adapter lifecycle persists a detached child PID, waits for health, writes state, re-probes health, and removes PID/state on startup failure, but no checked-in test exercises `ensureOpenRouterRuntimeAdapter` or the `cleanupFailedAdapterStartup` path.
  • Impact: A regression in startup failure cleanup could leave stale PID/state files or a ghost adapter process that later retries reuse incorrectly, breaking the deterministic lifecycle-state invariant that partial failures converge without ghost resources or stale ports.
  • Recommended action: Add direct lifecycle unit tests around `ensureOpenRouterRuntimeAdapter` by injecting or stubbing process, file, and health dependencies so failures after PID persistence and after state write can be simulated and cleanup assertions made.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search checked-in tests for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, or `probeAdapterHealth`; no matches are present, while `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274-295` contains the persist/wait/write/reprobe/cleanup sequence.
  • Missing regression test: A unit test that stubs or injects `spawnDetachedNodeAdapter`, `persistLocalAdapterPid`, `waitForLocalAdapterHealth`/`probeAdapterHealth`, `killLocalAdapterPid`, and state-file removal to prove failures after PID persistence and after state write clean up PID/state and that a subsequent retry cannot reuse the failed PID/state.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search checked-in tests for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, or `probeAdapterHealth`; no matches are present, while `src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274-295` contains the persist/wait/write/reprobe/cleanup sequence.
  • Evidence: Repository grep for `ensureOpenRouterRuntimeAdapter`, `cleanupFailedAdapterStartup`, `openrouter-runtime-adapter-lifecycle`, and `probeAdapterHealth` in `**/*test*.ts` returned no matches. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:274 persists the child PID before readiness is confirmed. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:282-289 writes adapter state and then re-probes health. src/lib/inference/openrouter-runtime-adapter-lifecycle.ts:293-295 calls `cleanupFailedAdapterStartup()` on errors, but that behavior is untested.

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.

@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: 1

🧹 Nitpick comments (5)
src/lib/inference/openrouter-runtime-adapter.ts (2)

39-39: 🚀 Performance & Scalability | 🔵 Trivial

Consider log rotation/size cap for the adapter's JSON-line log.

appendLocalAdapterJsonLine(LOG_PATH, ...) grows unbounded over the adapter's lifetime with no rotation or truncation policy visible in this file.

Also applies to: 66-79

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/inference/openrouter-runtime-adapter.ts` at line 39, The adapter’s
JSON-line log at LOG_PATH is appended to indefinitely, so add a rotation,
truncation, or size-cap policy around appendLocalAdapterJsonLine usage in
openrouter-runtime-adapter.ts. Update the logging flow in the openRouter runtime
adapter methods that write JSON lines (including the paths around the referenced
append calls) so the file is periodically rolled over or trimmed, and keep the
logging behavior encapsulated via a helper or shared utility to avoid unbounded
growth.

284-305: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant port re-validation.

OPENROUTER_RUNTIME_ADAPTER_PORT is already parsed and range-validated (1024–65535) via parsePort at module load using the same env var. Re-reading process.env.NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT here and re-validating with a weaker check (Number.isInteger(port) || port <= 0, no upper/lower bound) is duplicate logic that could simply reuse the constant.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/inference/openrouter-runtime-adapter.ts` around lines 284 - 305, The
startOpenRouterRuntimeAdapterFromEnv function is redundantly re-parsing and
re-validating the port instead of reusing the already validated
OPENROUTER_RUNTIME_ADAPTER_PORT constant. Update this function to rely on the
module-level parsed value and remove the extra process.env read plus the weaker
Number.isInteger/positive-only check, keeping the existing
createOpenRouterRuntimeAdapterServer and server.listen flow unchanged.
src/lib/inference/openrouter-runtime-adapter.test.ts (1)

45-101: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding coverage for upstream failure mid-stream.

No test exercises an upstream response that errors after headers are sent or hangs past a timeout — the exact gap tied to the missing error handler/timeout flagged in openrouter-runtime-adapter.ts. Adding such a test would catch regressions here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/inference/openrouter-runtime-adapter.test.ts` around lines 45 - 101,
Add a test in OpenRouter Runtime adapter coverage that exercises an upstream
failure after the response has started or a request that stalls until timeout,
since the current `createOpenRouterRuntimeAdapterServer` happy-path test does
not cover the missing error/timeout handling. Use the existing `upstream` server
setup and `listen` helpers to simulate a stream that emits partial data then
fails, or never finishes, and assert the adapter surfaces the failure instead of
hanging silently. Keep the assertions close to the existing `forwards chat
completions with OpenRouter attribution headers` test so regressions in
`openrouter-runtime-adapter.ts` are caught.
src/lib/onboard/inference-providers/remote.ts (1)

187-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hardcoded "openrouter-api" literal duplicates the OPENROUTER_PROVIDER_NAME constant.

openrouter-runtime.ts imports and checks against OPENROUTER_PROVIDER_NAME (from ../inference/openrouter), but this file re-derives the same provider name as a bare string literal. If the constant ever changes, openrouterCredentialValue would silently stop being hydrated for the real provider name while setupOpenRouterRuntimeInference's own guard uses the updated constant, causing a divergence that's easy to miss.

♻️ Suggested fix
+import { OPENROUTER_PROVIDER_NAME } from "../../inference/openrouter";
...
-  const openrouterCredentialValue =
-    provider === "openrouter-api" ? hydrateCredentialEnv(openrouterCredentialEnv) : null;
+  const openrouterCredentialValue =
+    provider === OPENROUTER_PROVIDER_NAME ? hydrateCredentialEnv(openrouterCredentialEnv) : null;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/inference-providers/remote.ts` around lines 187 - 189, The
provider check in `remote.ts` hardcodes the OpenRouter name instead of using the
shared `OPENROUTER_PROVIDER_NAME` constant. Update the
`openrouterCredentialValue` assignment to compare `provider` against that
imported constant, matching the guard used by `setupOpenRouterRuntimeInference`
so both paths stay in sync if the provider name changes.
src/lib/onboard/openrouter-runtime.ts (1)

135-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Redundant sandbox-update call — likely a no-op in production.

options.updateSandbox is typed as typeof registry.updateSandbox (the real module function taking (name, {model, provider})), but production wiring (remote.ts line 203) actually passes registry.updateSandbox destructured from RemoteProviderDeps, which resolves to the commonDeps.registry.updateSandbox wrapper defined in setup-inference.ts as (name: string) => reserveRoute(name, provider, model) — a single-argument function that ignores the {model, provider} object passed here. Since options.verifyInferenceRoute (line 123) already triggers reserveRoute and sets its internal routeReserved guard, this second call becomes a guaranteed no-op, making the block dead code that obscures actual behavior.

Consider removing this block, or aligning the updateSandbox type/wiring so it reflects what is actually invoked at runtime.

♻️ Suggested cleanup
-  if (options.sandboxName) {
-    (options.updateSandbox ?? registry.updateSandbox)(options.sandboxName, {
-      model: options.model,
-      provider: options.provider,
-    });
-  }
+  // Sandbox route is already reserved via options.verifyInferenceRoute above.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/openrouter-runtime.ts` around lines 135 - 140, The sandbox
update block in openrouter-runtime is effectively dead code because the
runtime-provided `options.updateSandbox` from `remote.ts`/`setup-inference.ts`
only accepts the sandbox name and just re-reserves the route, while
`options.verifyInferenceRoute` has already done that and set the guard. Remove
the `options.sandboxName` update call from `openrouter-runtime` or, if it must
remain, change the `RemoteProviderDeps`/`updateSandbox` wiring so
`registry.updateSandbox` truly performs the intended model/provider update
instead of a no-op wrapper.
🤖 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 `@src/lib/inference/openrouter-runtime-adapter.ts`:
- Around line 164-206: forwardOpenRouterRequest currently streams the upstream
response without handling upstreamRes errors and has no request timeout, leaving
the OpenRouter proxy path vulnerable to crashes or hangs. Update this function
to attach an error handler to upstreamRes alongside the existing upstreamReq
handler, ensure any upstream response failure either sends the JSON error or
destroys the client response safely, and add a timeout on the transport.request
call so stalled connections are aborted and resolved cleanly. Use
forwardOpenRouterRequest, upstreamReq, and upstreamRes to locate the logic.

---

Nitpick comments:
In `@src/lib/inference/openrouter-runtime-adapter.test.ts`:
- Around line 45-101: Add a test in OpenRouter Runtime adapter coverage that
exercises an upstream failure after the response has started or a request that
stalls until timeout, since the current `createOpenRouterRuntimeAdapterServer`
happy-path test does not cover the missing error/timeout handling. Use the
existing `upstream` server setup and `listen` helpers to simulate a stream that
emits partial data then fails, or never finishes, and assert the adapter
surfaces the failure instead of hanging silently. Keep the assertions close to
the existing `forwards chat completions with OpenRouter attribution headers`
test so regressions in `openrouter-runtime-adapter.ts` are caught.

In `@src/lib/inference/openrouter-runtime-adapter.ts`:
- Line 39: The adapter’s JSON-line log at LOG_PATH is appended to indefinitely,
so add a rotation, truncation, or size-cap policy around
appendLocalAdapterJsonLine usage in openrouter-runtime-adapter.ts. Update the
logging flow in the openRouter runtime adapter methods that write JSON lines
(including the paths around the referenced append calls) so the file is
periodically rolled over or trimmed, and keep the logging behavior encapsulated
via a helper or shared utility to avoid unbounded growth.
- Around line 284-305: The startOpenRouterRuntimeAdapterFromEnv function is
redundantly re-parsing and re-validating the port instead of reusing the already
validated OPENROUTER_RUNTIME_ADAPTER_PORT constant. Update this function to rely
on the module-level parsed value and remove the extra process.env read plus the
weaker Number.isInteger/positive-only check, keeping the existing
createOpenRouterRuntimeAdapterServer and server.listen flow unchanged.

In `@src/lib/onboard/inference-providers/remote.ts`:
- Around line 187-189: The provider check in `remote.ts` hardcodes the
OpenRouter name instead of using the shared `OPENROUTER_PROVIDER_NAME` constant.
Update the `openrouterCredentialValue` assignment to compare `provider` against
that imported constant, matching the guard used by
`setupOpenRouterRuntimeInference` so both paths stay in sync if the provider
name changes.

In `@src/lib/onboard/openrouter-runtime.ts`:
- Around line 135-140: The sandbox update block in openrouter-runtime is
effectively dead code because the runtime-provided `options.updateSandbox` from
`remote.ts`/`setup-inference.ts` only accepts the sandbox name and just
re-reserves the route, while `options.verifyInferenceRoute` has already done
that and set the guard. Remove the `options.sandboxName` update call from
`openrouter-runtime` or, if it must remain, change the
`RemoteProviderDeps`/`updateSandbox` wiring so `registry.updateSandbox` truly
performs the intended model/provider update instead of a no-op wrapper.
🪄 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: 1881170d-3967-4f8f-a910-ddc940009600

📥 Commits

Reviewing files that changed from the base of the PR and between 4970809 and 8b3dafe.

📒 Files selected for processing (17)
  • docs/inference/inference-options.mdx
  • docs/reference/commands.mdx
  • scripts/openrouter-runtime-adapter.js
  • src/lib/core/ports.test.ts
  • src/lib/core/ports.ts
  • src/lib/inference/openrouter-runtime-adapter.test.ts
  • src/lib/inference/openrouter-runtime-adapter.ts
  • src/lib/inference/openrouter.ts
  • src/lib/onboard.ts
  • src/lib/onboard/gateway-recovery.ts
  • src/lib/onboard/inference-providers/remote-openai-surface.test.ts
  • src/lib/onboard/inference-providers/remote.ts
  • src/lib/onboard/inference-providers/types.ts
  • src/lib/onboard/openrouter-runtime.ts
  • src/lib/onboard/setup-inference.ts
  • test/onboard-openrouter-inference.test.ts
  • test/support/setup-inference-test-harness.ts

Comment thread src/lib/inference/openrouter-runtime-adapter.ts Outdated
sandl99 added 3 commits July 9, 2026 22:08
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>

@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 `@src/lib/inference/openrouter-runtime-adapter-forward.ts`:
- Around line 82-96: The upstream error handler in
openrouter-runtime-adapter-forward.ts is forwarding err.message directly to the
client via sendJson and compactText, which can expose internal network details.
Update the upstreamReq.on("error") path to log the full error server-side and
return a generic client-facing message instead of the raw message, while keeping
the existing openrouter_runtime_error response shape and resolve(502) behavior.
Focus on the error handling block around upstreamReq.on("error") and
options.res.destroy so the response is sanitized regardless of whether headers
were already sent.
- Around line 61-103: Handle the missing failure paths in
forwardOpenRouterRequest by adding error and timeout handling around the
upstream stream lifecycle. In openrouter-runtime-adapter-forward.ts, update
forwardOpenRouterRequest so the upstreamRes.pipe(options.res) path listens for
upstreamRes errors and aborts/destroys the downstream response safely, and
configure transport.request with a timeout that rejects or destroys the upstream
request and returns the same 502 fallback. Keep the existing behavior for
buildUpstreamUrl, buildForwardRequestHeaders, and sendJson, but ensure both
stalled requests and mid-response resets are handled without crashing or
hanging.
🪄 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: f20f7916-2e5c-4f9f-b50c-1081d31a554c

📥 Commits

Reviewing files that changed from the base of the PR and between 8b3dafe and a77fa50.

📒 Files selected for processing (8)
  • src/lib/inference/openrouter-runtime-adapter-common.ts
  • src/lib/inference/openrouter-runtime-adapter-entry.ts
  • src/lib/inference/openrouter-runtime-adapter-forward.ts
  • src/lib/inference/openrouter-runtime-adapter-lifecycle.ts
  • src/lib/inference/openrouter-runtime-adapter-server.ts
  • src/lib/inference/openrouter-runtime-adapter.ts
  • src/lib/onboard/gateway-recovery.test.ts
  • src/lib/onboard/setup-inference-route-containment.test.ts

Comment thread src/lib/inference/openrouter-runtime-adapter-forward.ts Outdated
Comment thread src/lib/inference/openrouter-runtime-adapter-forward.ts Outdated
Comment thread src/lib/inference/openrouter-runtime-adapter-common.ts Fixed
Comment thread src/lib/inference/openrouter-runtime-adapter-common.ts Fixed

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/inference/openrouter-runtime-adapter-forward.ts (1)

155-171: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Upstream response error handling and request timeout still missing.

upstreamRes.pipe(options.res) has no error listener on upstreamRes, and there's no timeout on transport.request/upstreamReq. A mid-response reset from OpenRouter crashes the adapter process on the unhandled stream error, and a stalled upstream hangs the request indefinitely. This is the same issue raised in a prior review round on this file and remains unaddressed in the current diff.

🐛 Proposed fix
       (upstreamRes) => {
         const status = upstreamRes.statusCode || 502;
         options.res.writeHead(status, buildForwardResponseHeaders(upstreamRes.headers));
+        upstreamRes.on("error", (err) => {
+          resolve(sendForwardError(options.res, err));
+        });
         upstreamRes.pipe(options.res);
         upstreamRes.on("end", () => resolve(status));
       },
     );
+    upstreamReq.setTimeout(30_000, () => {
+      upstreamReq.destroy(new Error("OpenRouter upstream request timed out"));
+    });
     upstreamReq.on("error", (err) => {
       resolve(sendForwardError(options.res, err));
     });
     upstreamReq.end(body);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/inference/openrouter-runtime-adapter-forward.ts` around lines 155 -
171, In `openrouter-runtime-adapter-forward`’s upstream request flow, add proper
protection for both the response stream and stalled requests. Attach an error
handler to `upstreamRes` before piping to `options.res`, and ensure any stream
failure routes through `sendForwardError` instead of crashing the process. Also
set a timeout on `transport.request`/`upstreamReq` and abort/resolve with the
same forward error path when the upstream does not respond in time.
🤖 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 `@src/lib/inference/openrouter-runtime-adapter-forward.ts`:
- Around line 121-137: The generic error path in sendForwardError still forwards
err.message to the client, which can leak upstream/network details; update
sendForwardError to only expose the raw message for ForwardHttpError and use a
safe generic client-facing message for all other errors. Keep the existing
status/code handling in sendForwardError, but ensure compactText(message) is fed
a sanitized fallback string for non-ForwardHttpError cases rather than
err.message.

---

Outside diff comments:
In `@src/lib/inference/openrouter-runtime-adapter-forward.ts`:
- Around line 155-171: In `openrouter-runtime-adapter-forward`’s upstream
request flow, add proper protection for both the response stream and stalled
requests. Attach an error handler to `upstreamRes` before piping to
`options.res`, and ensure any stream failure routes through `sendForwardError`
instead of crashing the process. Also set a timeout on
`transport.request`/`upstreamReq` and abort/resolve with the same forward error
path when the upstream does not respond in time.
🪄 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: 682704e6-b69e-4fb2-ab37-2bc598823cd7

📥 Commits

Reviewing files that changed from the base of the PR and between a77fa50 and 380eb40.

📒 Files selected for processing (6)
  • src/lib/core/ports.test.ts
  • src/lib/core/ports.ts
  • src/lib/inference/openrouter-runtime-adapter-forward.ts
  • src/lib/inference/openrouter-runtime-adapter-lifecycle.ts
  • src/lib/inference/openrouter-runtime-adapter.test.ts
  • src/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/core/ports.test.ts
  • src/lib/onboard.ts
  • src/lib/inference/openrouter-runtime-adapter-lifecycle.ts

Comment thread src/lib/inference/openrouter-runtime-adapter-forward.ts
sandl99 added 2 commits July 9, 2026 22:28
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@cv cv added the v0.0.80 Release target label Jul 9, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes on exact head 183c3cbc8a0d09f53a88bae91753443332246b8c.

  1. In src/lib/inference/openrouter-runtime-adapter-forward.ts:155-171, add a bounded upstream timeout plus error and aborted handling for the upstream response, with single-settlement cleanup. Add regressions for a pre-response stall and a mid-response reset.
  2. In src/lib/inference/openrouter-runtime-adapter-forward.ts:121-129, do not return raw network error messages to clients. Return a stable generic response and keep diagnostic details server-side.
  3. Resolve the host-listener boundary. The adapter binds 0.0.0.0, /health bypasses auth, and runtime requests accept any Bearer-shaped value. The health payload itself is secret-free, and simply binding loopback is incompatible with the current direct host.openshell.internal route, so implement a topology-compatible source/auth/firewall restriction or record explicit maintainer risk acceptance with focused tests.
  4. After the source fixes, provide exact-head live evidence for inference-routing, network-policy, onboard-repair, onboard-resume, and cloud-onboard. Also add an OpenRouter-specific sandbox-to-adapter/upstream smoke proving route reachability, attribution headers, and denied unintended egress, or obtain an explicit maintainer waiver.

I am not treating the model-catalog item as part of this PR: that implementation is unchanged, this PR explicitly excludes it, and #5826 remains open. Please address the source blockers before spending live E2E capacity.

@sandl99

sandl99 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @cv, I'm requesting maintainer decision on HTTP injection. OpenRouter requested NemoClaw add custom HTTP headers as adverstise X-OpenRouter-Title. However, current architecture of OpenShell L7 proxy is limited and doesn't provide us a way to change that. We will have to wait until OpenShell provide NVIDIA/OpenShell#1738

There are two options.

  1. Keep this feature as blocked until OpenShell support HTTP injection
  2. Do this PR as a work around. Add a HTTP runtime adapter in front of OpenShell L7.

@sandl99

sandl99 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up pushed through a2a392a37.

What changed since the 183c3cb human review:

  • Added upstream response error/aborted handling plus upstream request timeout handling in openrouter-runtime-adapter-forward.ts.
  • Added regressions for both stalled upstream requests and mid-response upstream aborts in openrouter-runtime-adapter.test.ts.
  • Kept generic client-facing upstream failure messages; raw network details are not returned.
  • Bound runtime requests to the exact OpenShell-held Authorization value using a persisted SHA-256 authorization hash; arbitrary Bearer-shaped headers now return 401.
  • Switched OpenRouter onboarding back to the same featured cloud model picker/list as NVIDIA Endpoints, per feat: adding OpenRouter as an inference provider #5826.
  • Extracted OpenRouter uninstall cleanup into src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.ts.
  • Updated inference/platform docs and the PR body.

Local verification recorded in the PR body passed, and the latest pushed-head GitHub checks are green. CodeRabbit remains paused/skipped. The PR still shows CHANGES_REQUESTED from the older human review; the remaining item is maintainer re-review and a decision on the requested live E2E evidence/waiver for the exact head.

Comment thread src/lib/inference/openrouter-runtime-adapter-server.ts Fixed
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29070537794
Workflow ref: feat/openrouter-runtime-headers
Requested targets: (default — all supported)
Requested jobs: inference-routing
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
inference-routing ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29070537989
Workflow ref: feat/openrouter-runtime-headers
Requested targets: (default — all supported)
Requested jobs: onboard-repair
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
onboard-repair ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29070537970
Workflow ref: feat/openrouter-runtime-headers
Requested targets: (default — all supported)
Requested jobs: onboard-resume
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
onboard-resume ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29070537801
Workflow ref: feat/openrouter-runtime-headers
Requested targets: (default — all supported)
Requested jobs: cloud-onboard
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29070537946
Workflow ref: feat/openrouter-runtime-headers
Requested targets: (default — all supported)
Requested jobs: network-policy
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
network-policy ✅ success

@sandl99 sandl99 requested a review from cv July 10, 2026 05:34
@cv cv merged commit 0123ed9 into main Jul 10, 2026
434 checks passed
@cv cv deleted the feat/openrouter-runtime-headers branch July 10, 2026 17:33
cv pushed a commit that referenced this pull request Jul 11, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- #6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- #6584 / #6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- #6210 / #6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- #6624 / #6623 / #6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- #6629 / #6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- #6634 / #6626 / #6596 / #5569 / #6610 / #6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- #6578 / #5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


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

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…#6584)

<!-- markdownlint-disable MD041 -->
## Summary
OpenRouter runtime traffic now goes through a host-local NemoClaw
adapter so runtime requests include the OpenRouter attribution headers
that OpenShell cannot inject per provider today. The adapter keeps
`OPENROUTER_API_KEY` owned by OpenShell, binds runtime requests to the
exact OpenShell `Authorization` value via a persisted SHA-256 hash, and
adds only `HTTP-Referer` plus `X-OpenRouter-Title`.
This runtime adapter is a workaround until OpenShell exposes L7
middleware/default-header injection for provider routes.

## Related Issue
Fixes NVIDIA#5826.

## Changes
- Added an OpenRouter runtime adapter with split server, forwarding,
lifecycle, and compiled TypeScript entrypoint modules that forward
`/v1/chat/completions` to OpenRouter with attribution headers.
- Wired `openrouter-api` onboarding to register
`OPENAI_BASE_URL=http://host.openshell.internal:11437/v1` while
retaining `OPENROUTER_API_KEY` as the OpenShell provider credential.
- Added `NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT` with gateway and
adapter-port conflict validation.
- Hardened adapter lifecycle with a startup lock/post-persist health
probe, bounded request-body forwarding, exact bearer-token matching, and
hash-only adapter state.
- Kept OpenRouter on the same featured cloud model picker/list as NVIDIA
Endpoints while preserving OpenRouter-specific credential and
manual-model validation.
- Moved OpenRouter adapter uninstall cleanup into a focused module
instead of growing the uninstall run-plan monolith.
- Updated OpenRouter onboarding, adapter, provider recovery, port, and
docs coverage.

## Type of Change

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

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Local review completed
for the credential boundary. OpenShell still owns and injects
`OPENROUTER_API_KEY`; the adapter process receives only a SHA-256
authorization hash and does not receive the key in its environment,
state file, or token file. Adapter tests cover exact bearer-auth
enforcement, path containment, safe health output, oversized-body
rejection, generic upstream failure responses, and OpenRouter header
forwarding. The adapter intentionally binds `0.0.0.0` because the
current OpenShell container route reaches the host through
`host.openshell.internal`; exact bearer matching narrows runtime access
to the OpenShell-held credential value, and replacing the adapter with
OpenShell L7/default-header middleware is tracked as the TODO in
`src/lib/onboard/openrouter-runtime.ts`.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run --project cli src/lib/core/ports.test.ts
src/lib/inference/openrouter-runtime-adapter.test.ts
src/lib/onboard/inference-providers/remote-openai-surface.test.ts
src/lib/onboard/gateway-recovery.test.ts
src/lib/onboard/recovered-provider-reuse.test.ts
src/lib/inference/config.test.ts
src/lib/inference/onboard-probes.test.ts
src/lib/onboard/providers.test.ts src/lib/onboard/setup-nim-flow.test.ts
src/lib/onboard/setup-inference-route-containment.test.ts` passed; `npx
vitest run --project integration
test/onboard-openrouter-inference.test.ts` passed; `npm run
typecheck:cli` passed; `npm run build:cli` passed; `npm run
source-shape:check` passed; `npm run test-size:check` passed. Follow-up
hardening: `npx vitest run --project cli
src/lib/onboard/setup-nim-flow.test.ts
src/lib/inference/nvidia-featured-models.test.ts
src/lib/onboard/nvidia-featured-model-selection.test.ts
src/lib/inference/openrouter-runtime-adapter.test.ts
src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.test.ts
src/lib/actions/uninstall/run-plan.test.ts` passed; `npx vitest run
--project integration test/onboard-openrouter-inference.test.ts` passed;
`npm run typecheck:cli` passed; `npm run build:cli` passed; `npm run
docs` passed; `npm run test-size:check` passed; `npm run
test-conditionals:scan -- --top 25` passed; `npx tsx
scripts/check-env-var-docs.ts` passed; `git diff --check` passed.
Upstream-abort regression follow-up: `npx vitest run --project cli
src/lib/inference/openrouter-runtime-adapter.test.ts
src/lib/onboard/inference-providers/remote-openai-surface.test.ts
src/lib/onboard/setup-nim-flow.test.ts
src/lib/inference/nvidia-featured-models.test.ts
src/lib/onboard/nvidia-featured-model-selection.test.ts
src/lib/actions/uninstall/openrouter-runtime-adapter-cleanup.test.ts
src/lib/actions/uninstall/run-plan.test.ts` passed; `npx vitest run
--project integration test/onboard-openrouter-inference.test.ts` passed;
`npm run typecheck:cli` passed; `npm run build:cli` passed; `npm run
source-shape:check` passed; `npm run test-size:check` passed; `git diff
--check` passed.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] 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)

---
<!-- 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: San Dang <sdang@nvidia.com>


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

* **New Features**
* Added an OpenRouter runtime adapter with a public `/health` endpoint
and forwarding for `POST /v1/chat/completions` only, including
OpenRouter attribution headers.
* Onboarding can configure an OpenRouter-backed inference route and
support the new runtime adapter port/port offset.
* **Bug Fixes**
* Improved adapter request handling with body size enforcement and
explicit auth/endpoint gating.
* Strengthened port validation to reserve the OpenRouter adapter port
and prevent gateway conflicts (including recovery).
* **Documentation**
* Documented `NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`, validation
behavior, and runtime adapter health/log details.
* **Tests**
* Added adapter forwarding/auth gating/request-size tests and expanded
port validation coverage.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- NVIDIA#6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- NVIDIA#6584 / NVIDIA#6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- NVIDIA#6210 / NVIDIA#6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- NVIDIA#6624 / NVIDIA#6623 / NVIDIA#6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- NVIDIA#6629 / NVIDIA#6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- NVIDIA#6634 / NVIDIA#6626 / NVIDIA#6596 / NVIDIA#5569 / NVIDIA#6610 / NVIDIA#6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- NVIDIA#6578 / NVIDIA#5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


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

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs area: providers Inference provider integrations and provider behavior enhancement New capability or improvement request v0.0.80 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: adding OpenRouter as an inference provider

3 participants