Skip to content

fix(inference): close rejected Expect requests - #10283

Closed
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-rejected-expect-close
Closed

fix(inference): close rejected Expect requests#10283
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-rejected-expect-close

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

This follow-up to #10254 closes an incomplete client connection when the upstream server rejects an Expect: 100-continue request before accepting its body. The bridge now returns the upstream response, declares connection closure, and destroys the incoming request after the response finishes.

Tinson Lai remains the primary contributor for the continuation support merged in #10254. Apurv Kumaria authors only this post-merge connection-lifecycle repair.

Changes

  • Close an incomplete incoming request after an early upstream response finishes or closes.
  • Override the downstream Connection response header with close for this rejection path.
  • Extend the loopback 413 regression test to wait for server-side client closure while confirming that the upstream receives zero body bytes.

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:
  • 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: A current GitHub security review will be added after publication.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; this change does not modify scripts/prepare-dgx-station-host.sh.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

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 validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npm run test:changed passed 32 growth-guard tests and 21 affected bridge tests.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Not applicable. CLI type checking and repository checks passed for this two-file change.
  • 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: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of incomplete HTTP requests when upstream responses finish, close, or time out.
    • Prevented request bodies from being forwarded when Expect: 100-continue requests are rejected.
    • Added Connection: close to applicable rejection responses to ensure connections close cleanly.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria self-assigned this Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This repository limits you to 10 open pull requests. Please close or merge an existing PR before opening another one.

@github-code-quality

github-code-quality Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 6fc4683 in the codex/fix-rejected-e... branch remains at 96%, unchanged from commit 722fe87 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 6fc4683 in the codex/fix-rejected-e... branch remains at 83%, unchanged from commit 722fe87 in the main branch.

Show a line coverage summary of the most impacted files.
File main 722fe87 codex/fix-rejected-e... 6fc4683 +/-
src/lib/onboard...cker-journal.ts 75% 73% -2%
src/lib/onboard...er-gpu-patch.ts 81% 79% -2%
src/lib/onboard...eate-attempt.ts 90% 88% -2%
src/lib/onboard...trap/adapter.ts 71% 70% -1%
src/lib/onboard...ndbox-create.ts 90% 89% -1%
src/lib/onboard...uild-context.ts 74% 74% 0%
src/lib/onboard...eate-journal.ts 87% 90% +3%
src/lib/onboard...vider/docker.ts 85% 89% +4%
src/lib/onboard...er-gpu-route.ts 93% 97% +4%
src/lib/onboard...host-forward.ts 55% 84% +29%

Updated August 25, 2026 17:47 UTC

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4b988a37-1ec0-4055-b0f3-0a3505b9e3db

📥 Commits

Reviewing files that changed from the base of the PR and between 722fe87 and 6fc4683.

📒 Files selected for processing (2)
  • src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts
  • src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts

📝 Walkthrough

Walkthrough

The bridge now destroys incomplete client requests after response completion or closure. It closes rejected Expect: 100-continue connections without forwarding request bodies. Tests can wait for server-side closure and verify Connection: close.

Changes

Request cleanup

Layer / File(s) Summary
Incomplete request cleanup
src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts
The bridge pauses and destroys incomplete requests after response completion or closure. It closes rejected Expect: 100-continue connections and reuses the cleanup helper after continue timeouts.
Connection closure validation
src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts
The test request helper can wait for server-side closure. The upstream rejection test verifies Connection: close.

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

Suggested reviewers: laitingsheng, prekshivyas

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-rejected-expect-close

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

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

This follow-up remains required after #10254 merged.

The repository closed this PR through the 10-open-PR policy. Apurv Kumaria has 10 open PRs:

I will not close an unrelated or excluded PR to free a slot. A human must merge or close an eligible Apurv PR, or apply the repository's documented exemption policy, before this PR can reopen.

The repair commit is GitHub Verified and includes DCO sign-off. Local evidence passes 32 growth-guard tests, 21 affected bridge tests, CLI type checking, repository checks, and normal hooks.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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
Synthesis status: Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions

E2E guidance

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

Recommended E2E: managed-image-protected-runtime

Manual-only E2E: managed-image-multiarch-startup, onboard-repair, onboard-resume, cloud-onboard
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant