This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

No actionable comments were generated in the recent review. 🎉

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 38d4854f-04c8-43f3-9587-3567b54fdc34

Reviewing files that changed from the base of the PR and between ece6a7a and b42dd2c.

• src/lib/inference/bedrock-runtime-adapter.test.ts
• src/lib/inference/bedrock-runtime-adapter.ts
• src/lib/inference/local-adapter-lifecycle.test.ts
• src/lib/inference/local-adapter-lifecycle.ts
• src/lib/inference/openrouter-runtime-adapter-lifecycle.ts
• src/lib/inference/openrouter-runtime-adapter.test.ts

• src/lib/inference/openrouter-runtime-adapter-lifecycle.ts
• src/lib/inference/bedrock-runtime-adapter.ts
• src/lib/inference/local-adapter-lifecycle.ts
• src/lib/inference/openrouter-runtime-adapter.test.ts
• src/lib/inference/bedrock-runtime-adapter.test.ts
• src/lib/inference/local-adapter-lifecycle.test.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 6 remain after this review.

Walkthrough

The change adds spawned-PID fallback cleanup for failed local adapter startups. Bedrock and OpenRouter now use shared lifecycle configuration and cleanup helpers. Tests cover failed health checks, PID persistence errors, process termination, and state-file removal.

Changes

Adapter lifecycle management

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b42dd

The change cleans up failed Bedrock adapter startups while preserving the existing error behavior, with targeted tests and validation passing; no actionable merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant AdapterStartup
  participant LocalAdapterLifecycle
  participant AdapterProcess
  AdapterStartup->>AdapterProcess: Spawn child
  AdapterStartup->>LocalAdapterLifecycle: Persist PID and wait for health
  LocalAdapterLifecycle-->>AdapterStartup: Startup failure
  AdapterStartup->>LocalAdapterLifecycle: Cleanup with spawned PID
  LocalAdapterLifecycle->>AdapterProcess: Terminate child
  LocalAdapterLifecycle-->>AdapterStartup: Remove PID and state files

❌ Failed checks (1 warning)

• Create PR with unit tests

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

Pass the spawned PID to failed-startup cleanup — this is right, and it was a hole in the fix
itself rather than a pre-existing one.

persistLocalAdapterPid is the first statement inside the guarded block and it can throw on its own.
It goes through writePrivateLocalAdapterFile, which creates the state directory, rejects a symlinked
path, refuses a non-file target, then opens and writes the file; a full or read-only filesystem, a
state path that is not a directory, or a path swapped underneath it all raise there. The pre-spawn
killLocalAdapterPid has already removed the pid file by that point, so when the persist throws there
is nothing left for the cleanup to reload — it removed the state file and left the child holding the
port, which is exactly the outcome this change exists to prevent.

killLocalAdapterPid now takes the pid as an optional second argument that defaults to reloading
pidPath, cleanupFailedLocalAdapterStartup forwards it, and both the Bedrock and OpenRouter catch
blocks pass child.pid. spawnDetachedNodeAdapter returns the ChildProcess synchronously, so
child.pid is populated before the guarded block runs; detached and unref() change process-group
membership and event-loop refcounting, not the pid, so it is the same value the pid file would have
held. When child.pid is undefined — a spawn that never produced a process — the default applies
and behaviour is unchanged. On the health-timeout path the persisted pid and child.pid are the same
value, so that path is unchanged as well.

HTTPS Pin is still deliberately not routed through the shared helper. Its killStaleAdapter is async,
awaits process exit, and throws rather than let a competing listener start; the shared helper is
synchronous, so routing it there would silently drop that guarantee and would save no lines.

Two regression cases, both failing before the change:

• local-adapter-lifecycle.test.ts drives the real cleanupFailedLocalAdapterStartup with no pid file
  on disk — the state a failed persist leaves behind. Without a spawned pid nothing is signalled (the
  control); with one, kill <pid> is issued and the state file is removed.
• bedrock-runtime-adapter.test.ts makes persistLocalAdapterPid throw, then asserts the original
  error propagates unchanged and that the spawned pid reaches the cleanup.

Add an OpenRouter lifecycle cleanup test — also right. ensureOpenRouterRuntimeAdapter had no test
call site anywhere in the repo, so the four wrappers this change removes from that lifecycle were
replaced without coverage.

openrouter-runtime-adapter.test.ts now drives a startup that never becomes healthy and asserts
against the real shared cleanup rather than a mock of it: the spawned child is signalled exactly once
and both openrouter-runtime-adapter.pid and openrouter-runtime-adapter.json are gone afterwards.
The adapter's pid, state and lock paths are redirected into a temporary directory for the test and the
process runner is stubbed, so nothing outside the test writes to disk and no real signal is sent.
Deleting the cleanupFailedLocalAdapterStartup call from that catch block makes the new case fail, so
it pins the behaviour rather than passing through.

Measured against the merge base, the pull request is now +207 / -65 across six files; production alone
is +69 / -64. The changed test files stay well inside ci/test-file-size-budget.json (526, 474 and
418 lines against the 1500 default, none of them carrying a legacy entry) and add no branch or loop,
so Codebase growth guardrails and Source-shape test budget both pass.

Checks run locally: src/lib/inference under the cli project is 2072 passed with the same 13
pre-existing failures the base has (network- and Docker-dependent suites, none of them in the files
touched here); npm run typecheck:cli and npm --prefix nemoclaw run typecheck are clean; npx oxlint on all six changed files exits 0; npx prek run --from-ref upstream/main --to-ref HEAD is
Passed or Skipped for every hook with nothing left to autofix; npm run validate:pr exits 0 with no
failing hook.

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized terminology decisions match; normalized E2E selections differ; severity counts match.

Advisory only. The primary lane did not select these E2E jobs or targets.

• bedrock-runtime-compatible-anthropic: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: inference-routing

Manual-only E2E: network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Maintainer final review: PASS

Correctness

• Failed Bedrock startup now cleans up the newly spawned child and stale PID and state files before rethrowing the original error.
• The shared helper replaces equivalent OpenRouter cleanup and accepts the spawned PID so cleanup still works when PID-file persistence fails.
• Process matching remains mandatory before signaling a PID. The HTTPS-pin adapter keeps its separate asynchronous stop contract.
• Focused coverage includes unhealthy startup, PID persistence failure, shared cleanup, and the unchanged OpenRouter behavior.

Security review: PASS

• Input and command handling: PASS — the kill argument is a numeric spawned or persisted PID and process identity is checked before signaling.
• Secrets and diagnostics: PASS — no credential value or new diagnostic is exposed; the random adapter token remains owner-only and is replaced on the next successful start.
• Network and SSRF: PASS — endpoint classification and request behavior do not change.
• Authentication and authorization: PASS — provider credentials and adapter token authority do not change.
• Dependencies and artifacts: PASS — no dependency, image, workflow, or artifact change.
• Sandbox and host boundary: PASS — cleanup is limited to the matching host adapter process and its adapter-owned PID and state files.
• Failure behavior: PASS — partial startup is cleaned up and the original failure is rethrown unchanged.
• Concurrency and lifecycle: PASS — existing locking and adapter-specific lifecycle contracts remain in place.
• Data integrity and persistence: PASS — cleanup removes only PID and state files; reusable credentials and unrelated adapter files are not deleted.

Compliance, documentation, and size

• No documentation change is required because command behavior and user-facing errors are unchanged.
• All required checks pass, the PR is approved and mergeable, and no unresolved review threads remain.
• DCO passes, every branch commit is GitHub-verified, and the original contributor attribution remains.
• Net change is +207/-65 across six focused implementation and test files. This is not a large change.

CI blocker update

The required CLI aggregate remains failed because the unchanged setup-nim-flow serving-profile test exceeded its five-second limit again in shard 1. The PR does not change that test or its code path. The same focused test passes locally (3/3), the PR's focused tests pass, and repository validation passes.

Because the same required job has now repeated the timeout across bounded reruns, I am not submitting another blind retry. The implementation, security review, documentation review, DCO, commit verification, and approval are complete. The remaining blocker is the required CI test timing out on hosted capacity; it needs CI recovery or maintainer direction before merge. The optional Nemotron advisor failure did not report an actionable code finding.