Skip to content

chore(build): lint raw TcpStream::connect - #5280

Merged
TaylorMutch merged 5 commits into
NVIDIA:mainfrom
ericcurtin:chore/2577-nodelay-dial-lint/ericcurtin
Nov 10, 2026
Merged

chore(build): lint raw TcpStream::connect#5280
TaylorMutch merged 5 commits into
NVIDIA:mainfrom
ericcurtin:chore/2577-nodelay-dial-lint/ericcurtin

Conversation

@ericcurtin

@ericcurtin ericcurtin commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fail clippy on raw TcpStream::connect so dials keep TCP_NODELAY.

Related Issue

Closes #2577

Changes

  • Add clippy.toml disallowing tokio and std TcpStream::connect.
  • connect_tcp_nodelay_best_effort takes any ToSocketAddrs; add a std twin.
  • Reroute dials to the helpers and drop duplicate nodelay calls.
  • Allow raw dials only in sandbox tests and probes, and in e2e.
  • Note the lint in AGENTS.md.

Testing

  • cargo clippy --workspace --all-targets, sandbox perf-harness and e2e/rust clean
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown

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.

@ericcurtin

ericcurtin commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

@purp @TaylorMutch PTAL when you get a chance, and /ok to test 92794dfe155467b0dfd679fd646e61962650c99b if it looks good. Review Readiness only fails for the missing area:build label, which I cannot add. Thank you!

@TaylorMutch
TaylorMutch requested a review from purp November 10, 2026 08:58
@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 92794df

@TaylorMutch TaylorMutch added the test:e2e Requires end-to-end coverage label Nov 10, 2026
@github-actions

github-actions Bot commented Nov 10, 2026

Copy link
Copy Markdown

Label test:e2e applied for 92794df. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Pushed a54e36c to fix a missed raw dial caught during a local review check: the Windows-only host proxy test in crates/openshell-supervisor-network/src/host.rs still called TcpStream::connect, which the new Clippy rule rejects under --all-targets -- -D warnings.

The test now uses connect_tcp_nodelay_best_effort, and the unused TcpStream import is removed.

Local validation: rustfmt and git diff --check pass. An isolated Clippy probe confirmed that both configured raw dial methods are rejected, including from a nested workspace. The Windows lint task was not run locally; Windows validation remains outstanding.

@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test a54e36c

@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test a8ff70e

TaylorMutch
TaylorMutch previously approved these changes Nov 10, 2026
View reviewed changes

@TaylorMutch TaylorMutch 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.

Reviewed the changes against issue #2577. The missed Windows host-proxy test dial and missing SPDX header have been fixed. All current checks pass, including the Fedora rootless Podman conformance suite on retry.

@TaylorMutch
TaylorMutch added this pull request to the merge queue Nov 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 10, 2026
@TaylorMutch
fix(build): reconcile TCP dial callers after merging main
0056959 
Merge main at 22209de and migrate the new test callers to the nodelay helper's address contract.

Signed-off-by: Taylor Mutch <[email protected]>
@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 0056959

@TaylorMutch

TaylorMutch commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

/ok to test c119cb5

TaylorMutch
TaylorMutch approved these changes Nov 10, 2026
View reviewed changes

@TaylorMutch TaylorMutch 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.

Re-reviewed after merging main and fixing the additional TCP dial callers and standalone test-support lint exception. Local affected-crate and test-support Clippy checks, focused startup and TCP_NODELAY tests, and the PostgreSQL-backed startup test pass. All current CI checks pass on c119cb5.

@TaylorMutch
TaylorMutch added this pull request to the merge queue Nov 10, 2026
Merged via the queue into NVIDIA:main with commit 0603b15 Nov 10, 2026
152 checks passed
@matthewgrossman matthewgrossman mentioned this pull request Nov 10, 2026
Merged
6 of 7 tasks
@rhods-devops-app rhods-devops-app Bot mentioned this pull request Nov 11, 2026
Open
jgarciao pushed a commit to jgarciao/OpenShell that referenced this pull request Nov 11, 2026
@ericcurtin @TaylorMutch @jgarciao
chore(build): lint raw TcpStream::connect (NVIDIA#5280)
c5a3bbd 
* chore(build): lint raw TcpStream::connect

Signed-off-by: Eric Curtin <[email protected]>

* fix(network): use nodelay helper in Windows host proxy test

Signed-off-by: Taylor Mutch <[email protected]>

* chore(build): add SPDX header to Clippy configuration

Signed-off-by: Taylor Mutch <[email protected]>

* fix(test): document standalone WebSocket dial lint exception

Signed-off-by: Taylor Mutch <[email protected]>

---------

Signed-off-by: Eric Curtin <[email protected]>
Signed-off-by: Taylor Mutch <[email protected]>
Co-authored-by: Taylor Mutch <[email protected]>
@HarryMoss HarryMoss mentioned this pull request Nov 12, 2026
Open
4 of 6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TaylorMutch TaylorMutch TaylorMutch approved these changes
@purp purp Awaiting requested review from purp

Assignees

No one assigned

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(build): enforce TCP_NODELAY at TCP dial sites with clippy::disallowed_methods

2 participants

@ericcurtin @TaylorMutch