fix(inference): bound adapter health-response memory

[HOYALIM]

Summary

Local inference adapter health checks currently retain an unbounded loopback response before validating its token hash. Bound retained health-response data to 64 KiB and fail closed on oversized declared or chunked bodies, so a faulty adapter or port collision cannot grow CLI memory with the response size.

Changes

• define a shared 64 KiB health-response budget for local inference adapters
• reject oversized Content-Length values before buffering response data
• stop chunked responses as soon as their cumulative size exceeds the budget
• avoid copying incoming Buffer chunks and settle every terminal path once
• cover oversized declared and chunked responses through the real loopback probe path

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: internal health-probe resource bounds only; provider configuration and CLI output are unchanged
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: fix(inference): bound adapter health-response memory #7661 (review) at fbac40b0; all nine categories pass; exact head 42224f624 revalidated after early-close correction
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: src/lib/inference/local-adapter-lifecycle.ts bounds internal health responses; src/lib/inference/local-adapter-lifecycle.test.ts covers oversized chunked and declared responses; no user-facing contract or output changed
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line 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 — npx --no-install vitest run --project cli src/lib/inference/local-adapter-lifecycle.test.ts (15 passed)
• Applicable broad gate passed — npm run checks; npm run build:cli
• 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: Ho Lim subhoya@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Improved local adapter health checks to reject oversized health responses.
  • Health checks now enforce a maximum response size, with safe failure on streaming, size-limit breaches, and parsing issues.
  • Prevented duplicate or conflicting health-check outcomes.
• Tests
  • Added lifecycle coverage to verify health probing fails closed when the health payload exceeds the configured byte limit (including chunked/streamed and content-length overflow cases).
  • Added coverage for accepting a valid health response exactly at the byte limit.

[copy-pr-bot]

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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

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

The local adapter health probe caps responses at 64 KiB, handles oversized or failed streams as unhealthy, prevents multiple settlements, and adds boundary tests for streamed, exact-limit, declared oversized, and early-close responses.

Changes

Local adapter health response bounding

Layer / File(s)	Summary
Enforce bounded health responses src/lib/inference/local-adapter-lifecycle.ts, src/lib/inference/local-adapter-lifecycle.test.ts	The probe enforces the response-size cap using content-length and streamed byte counts, destroys oversized responses, settles only once, handles failed or incomplete responses as unhealthy, and tests boundary cases.

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

Sequence Diagram(s)

sequenceDiagram
  participant HealthServer
  participant probeLocalAdapterHealth
  participant ResponseStream
  HealthServer->>probeLocalAdapterHealth: health response
  probeLocalAdapterHealth->>ResponseStream: inspect content-length and data chunks
  ResponseStream-->>probeLocalAdapterHealth: streamed response data or close event
  probeLocalAdapterHealth->>ResponseStream: destroy on size limit breach
  probeLocalAdapterHealth-->>HealthServer: resolve health result

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

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 E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy

Workflow run details

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/lib/inference/local-adapter-lifecycle.ts (1)

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

Use close instead of deprecated aborted. Keep the !res.complete guard so incomplete responses still settle as unhealthy.

🤖 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/local-adapter-lifecycle.ts` at line 235, In the response
lifecycle handler around settle, replace the deprecated res “aborted” event with
the “close” event and retain the !res.complete guard so only incomplete
responses settle as unhealthy.

Source: Learnings

🤖 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/local-adapter-lifecycle.test.ts`:
- Around line 160-174: Update the oversized-response test around
probeLocalAdapterHealth so the server sends valid JSON containing the expected
token hash while still exceeding LOCAL_ADAPTER_HEALTH_MAX_RESPONSE_BYTES, rather
than whitespace-only bytes. Keep the assertion that probeLocalAdapterHealth
resolves false, ensuring it fails if the response-size cap is removed.

---

Nitpick comments:
In `@src/lib/inference/local-adapter-lifecycle.ts`:
- Line 235: In the response lifecycle handler around settle, replace the
deprecated res “aborted” event with the “close” event and retain the
!res.complete guard so only incomplete responses settle as unhealthy.

🪄 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: 731dd397-bc46-437a-9398-8d8ae7f4c99f

📥 Commits

Reviewing files that changed from the base of the PR and between f6b0c98 and f3ebdf9.

📒 Files selected for processing (2)

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

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[prekshivyas]

Maintainer security review: PASS at exact PR SHA fbac40b04e08bb6ab678ceaef4b3cf60c2112abc.

1. Secrets and credentials — PASS: no credential value is added, retained, or logged; token verification still compares only the SHA-256 hash.
2. Input validation — PASS: declared and streamed response sizes are capped at 64 KiB before JSON parsing, and oversized responses fail closed.
3. Authentication and authorization — PASS: the expected-token-hash contract is unchanged, and a malformed or oversized response cannot become healthy.
4. Dependencies — PASS: no dependency changed.
5. Error handling and logging — PASS: timeout, response abort/error, parse failure, and request error settle once without exposing response contents.
6. Cryptography and data protection — PASS: the existing SHA-256 token hashing remains unchanged; this PR adds no cryptographic behavior.
7. Configuration and security headers — PASS: no port, origin, image, permission, or runtime configuration changed.
8. Security testing — PASS: focused coverage now rejects oversized chunked and declared Content-Length responses; npx vitest run --project cli src/lib/inference/local-adapter-lifecycle.test.ts passed 15/15.
9. System security — PASS: retained health-response memory is bounded, buffers are released on overflow, and terminal paths cannot overwrite the first verdict.

No correctness or security findings remain. The CodeRabbit aborted-event note is a non-blocking API-maintenance suggestion; it does not weaken the bounded-memory or fail-closed behavior.

[prekshivyas]

Exact-head ordinary CI is green, but credentialed E2E is blocked by the repository's fork-approval environment configuration.

• Head: fbac40b04e08bb6ab678ceaef4b3cf60c2112abc
• Reviewed base for this plan: f6b0c980eed1d1fc34874216fe0d26aee61fd84c
• Controller: https://github.com/NVIDIA/NemoClaw/actions/runs/30422111700
• Selected plan: inference-routing, network-policy

The controller refused to start the plan because approve-credentialed-e2e-for-fork-pr has no required-reviewer protection rules, so GitHub recorded no protected approval. No selected E2E ran and no repository credential was exposed. A repository admin must configure required reviewers; the workflow then requires a new PR head and fresh CI. I have not weakened or changed the environment.

[cjagwani]

Exact-head maintainer security review — PASS at 15ec6b0c0675fce17f2e398d96af0960ae754530 against base 795de982e26b400009bf1fc0f3826845d79acc4d.

1. Secrets and credentials: PASS. Response bodies are never logged, and the existing expected-token-hash comparison remains the only health identity check.
2. Input validation: PASS. Both declared and streamed bodies are capped at 64 KiB; oversized, malformed, incomplete, aborted, and errored responses fail closed.
3. Authentication and authorization: PASS. No permission or trust boundary changes; an oversized or partial response cannot be classified healthy.
4. Dependencies and supply chain: PASS. No dependency, lockfile, image, or download changes.
5. Error handling and exposure: PASS. A single-settlement guard prevents later events from overwriting the first terminal verdict without exposing response data.
6. Cryptography: PASS. Existing SHA-256 token hashing is unchanged.
7. Configuration and infrastructure: PASS. No ports, origins, policies, filesystem permissions, or runtime configuration change.
8. Security tests: PASS. Exact-head tests cover streamed overflow, declared overflow before buffering, the exact boundary, incomplete close, and valid/invalid token hashes. Required CI and protected E2E are green.
9. System security: PASS. Retained loopback health data is bounded; overflow clears retained chunks and destroys the response.

All 10 commits are GitHub Verified, the current primary advisor reports 0 blockers / 0 warnings / 0 suggestions, and no unresolved current review thread remains. Documentation-writer result is no-docs-needed at this exact head. Diff fingerprint: 58fb9b8d1917563d2dc993efe7d1de8cc685ff7acb41c9a1b12bb29b260dc0be.

[prekshivyas]

Approved exact head 15ec6b0. All 40 current checks are green; trusted E2E passed inference-routing and network-policy against base 795de98; focused health-probe suite passed 17/17; all commits are GitHub Verified; docs review is no-docs-needed; security review passes all nine categories; no unresolved major/critical findings. GitHub reports MERGEABLE/CLEAN. Per maintainer direction, a newer conflict-free main SHA alone does not require another refresh.

[prekshivyas]

Maintainer coordination: exact-head review, CI, and deterministic E2E are in progress for eec44b9e35c4b0a3df00f7b31045311da7ab697c. The two-file product patch remains byte-identical across the recent base merges (5f7251661da84232e73e97831d537587a425a97b54ff892ee70b9ad0b4a195d7), and GitHub reports MERGEABLE with no conflict. Please do not merge main into this branch again solely for base currency; each such refresh invalidates exact-head CI/E2E evidence. Refresh only if an actual conflict or behavioral reason appears.

[cjagwani]

Exact-head approval after the branch reached the quiet threshold. The production and regression-test patch is byte-for-byte unchanged from previously security-reviewed head 15ec6b0; intervening commits only refresh main and exact-head gate evidence. Deterministic maintainer gate passes with all 40 current checks green, clean merge state, all 14 commits Verified, protected inference-routing and network-policy evidence, and zero advisor findings. The prior nine-category security PASS therefore remains applicable to this exact product diff.

[prekshivyas]

Approved exact head da824e96f776672036cf0fd93dc6f64c8283148f. This head adds only a signed/Verified empty gate-refresh commit over the reviewed two-file product patch; its product tree is byte-identical to eec44b9e35c4b0a3df00f7b31045311da7ab697c. The hard gate reports allPass: true: all 40 current checks are green; trusted inference-routing and network-policy E2E passed against base 376beb50b6d184675283bdbc4f2eca18d7200a86; focused tests passed 17/17; all 14 commits are Verified; exact-head documentation writer result is no-docs-needed; nine-category security review passes; no unresolved threads; GitHub reports CLEAN/MERGEABLE. No conflict-based refresh is needed.

[prekshivyas]

Maintainer coordination: please do not merge main into this branch again solely for base currency. The user explicitly waived conflict-free refreshes. Each such merge invalidates exact-head CI/E2E and documentation receipts; refresh only if GitHub reports an actual conflict or reviewed behavior requires it. I am preparing one final receipt-first gate head now.

[prekshivyas]

Maintainer approval for exact head 42224f6. The two-file inference patch remains byte-identical after the signed/Verified base merge and empty gate-refresh commit. All 40 current checks pass; trusted E2E inference-routing and network-policy both pass; focused adapter lifecycle tests pass 17/17; all 16 commits are GitHub Verified; the exact-head documentation-writer receipt is no-docs-needed; the nine-category security review passes; and there are no unresolved review threads. The PR is CLEAN and MERGEABLE. Current main advanced only through #7864 with zero path overlap, and git merge-tree produced a clean tree; per the maintainer direction, no conflict-free currency refresh is required.