Streaming APIs (phase 2): stream()/start() terminals on the fluent WinRMClient - #134
Conversation
…ders (#111) WQL queries and remote commands can now be consumed incrementally, as the WSMan responses arrive, with memory bounded by one page/chunk instead of the whole result: - WqlRequest.stream() returns a closeable, lazy Stream<WqlRow>: rows are yielded as soon as they are parsed and the next WS-Enumeration Pull is issued only as the stream advances. Closing early sends a WS-Enumeration Release (new envelope) so the server frees the enumeration context; exhaustion releases the connection on its own. - CommandRequest.start() returns a Process-like RemoteProcess: stdout() and stderr() readers decoded incrementally with a stateful CharsetDecoder (multibyte characters split across Receive responses decode correctly), waitFor()/waitFor(Duration), exitCode(), and close() sending the terminate Signal that stops a still-running remote command. - CommandRequest.onStdout()/onStderr() callbacks tail the output live while execute() keeps its blocking contract and complete CommandResult. For the streaming terminals the request timeout is an inactivity timeout (longest tolerated server silence, enforced per round trip via the WSMan OperationTimeout and the socket read timeout), not an overall deadline; an open stream/process pins the client's serial connection until closed, like a JDBC ResultSet. Plumbing: WsmanClient now exposes stream-first primitives (WqlEnumeration, RemoteCommand) and the blocking wql()/executeCommand() are drains over them, so the two paths cannot drift; the connection lock became a Semaphore so a handle can be closed from another thread; WindowsRemoteExecutor gained default streamWql()/startCommand() methods returning the new public WqlCursor/CommandCursor SPI types, implemented by LightWinRMService. Closes #111 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e5bfe79f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ed RemoteProcess inert (Codex P2s) - HttpTransport gains inactivityTimeout(int): for streaming opens the socket read timeout IS the inactivity bound, with none of the +10s headroom the blocking paths keep for the retryable op-timeout fault — a server that stops answering entirely is now detected at the configured timeout, not ten seconds later. Regression test with a response delayed far past the timeout, asserting the elapsed detection time. - Closing a RemoteProcess early now transitions the local state: the decoders are flushed (buffered output stays readable, then EOF) and no read/wait may ever call cursor.next() again — the cursor released the connection, so the previous behavior could fire a stray Receive without owning it and race a later operation. The RemoteCommand primitive gets the same guard (a finished handle always yields null). waitFor()/exitCode() after an early close throw IllegalStateException — unless the command's final chunk had in fact been received, in which case close() recovers the real exit code from the cursor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e126f18fc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rs after close (Codex P2s) - New WsmanClient.exchange(): every round trip of a streaming operation — the initial Enumerate, shell Create, Command, each Pull and Receive — now translates the two "server stayed quiet" signals (socket read timeout and the WSMan op-timeout fault 2150858793) into the documented inactivity TimeoutException, so stream()/start() surface WinRMTimeoutException no matter which request exceeded the limit first. Blocking mode goes through the unchanged expectOk() path. - WsmanClient.close() now sets a closed flag before hard-closing the transport, and request() refuses to run on a closed client: a streaming handle (or abandoned worker) outliving the client can no longer trigger a transparent reconnect + re-authentication that would leak a socket nothing ever closes. The handle cleanup paths (terminate Signal, WS-Enumeration Release) skip their network send in that state and just release the connection permit. Regression tests: op-timeout fault answering the initial Enumerate / shell Create → WinRMTimeoutException; client closed while a stream/process is open → closing the handle stays silent on the wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The CLI now dogfoods the fluent WinRMClient streaming terminals: - wql streams each row to stdout (flushed per row) as the WS-Enumeration pages arrive: a huge query starts producing JSON Lines immediately and memory stays bounded by one page. The -t timeout is the stream's inactivity timeout (longest tolerated server silence), so large results are no longer killed by an overall deadline; a mid-stream failure can leave partial output before the nonzero exit (documented). - command/exec forwards stdout and stderr chunks live to the local streams while the remote command runs (onStdout/onStderr + execute(), which keeps the overall wall-clock deadline and the complete exit-code contract). - connect() now builds a WinRMClient: --https-permissive maps to the per-client trustAllCertificates() instead of mutating the global org.metricshub.winrm.tls.insecure system property (the ambient property still works for those who set it themselves); WinRMTimeoutException maps to exit code 124 like the legacy TimeoutException. The RemoteOperations test seam became consumer-based, and the Windows code-page decoding test now runs the full CLI stack (argument parsing, real connect factory, streaming forwarders) against FakeWsmanServer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a334905cf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(Codex P2) waitFor(Duration) could overshoot by a whole inactivity timeout: the loop only checked the clock between round trips, and a silent command left the wait blocked in a Receive bounded by the (much larger) per-response timeout. The remaining wait now bounds the round trip itself, using the protocol's own mechanism: CommandCursor gains poll(maxWaitMillis) (default: next()), backed by RemoteCommand.pollChunk, which sends the Receive with OperationTimeout = remaining wait. A compliant server answers within that — with output, or with the "nothing yet" op-timeout fault, which the poll returns as an EMPTY chunk instead of failing: an expired bounded wait is not an inactivity timeout, the connection stays in sync (the socket keeps the fault-wins headroom on purpose), and the process remains fully usable. Regression test: a wait of 200 ms against a silent command sends a Receive whose OperationTimeout is sub-second (not the 10 s inactivity bound), returns false on the fault, and the process then completes normally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 664cc3f5d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…er (Codex P2) The bounded poll's socket read timeout no longer reuses the blocking paths' ten-second fault headroom: HttpTransport.pollTimeout(wait) caps the read at wait + 1 s — enough for a live server's "nothing yet" op-timeout fault (generated the instant the requested wait expires) to cross the network and win the race, small enough that a peer that stopped answering entirely cannot hold waitFor(Duration.ofMillis(200)) hostage for ten seconds. When even that expires, the Receive was abandoned mid-flight, so the transport is dropped outright before reporting the timeout: a late response must not be readable as the answer to a later request — the next operation (e.g. the close-path Signal) reconnects and re-authenticates cleanly. Regression test: a peer answering 4 s late fails a 200 ms wait in ~1.2 s (not 10.2 s), and close() still terminates the command over a fresh connection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 978f5c0985
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… P2) A bounded poll's socket timeout was per-HTTP-round-trip: when the poll had to re-establish the connection, the NTLM probe, the challenge exchange and the final Receive each got a fresh wait+1s, letting a slow peer stretch a deadline-bounded waitFor(Duration) to several multiples of the requested wait. HttpTransport.pollTimeout(wait) now also records an absolute deadline (now + wait + headroom); every socket wait until the next timeout-mode switch — the reconnect's connect timeout, and each leg's read timeout, re-capped at the start of every post() — gets only what is left of that budget (1 ms floor, so an expired deadline fails fast instead of turning into an infinite socket wait). The other timeout modes clear the deadline. Unit test: a server answering every leg after 600 ms (fast enough for any single leg) against a 500 ms poll — the third leg must run out of the shared 1.5 s budget instead of the sequence running to 8 x 600 ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4563e72248
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…l (Codex P2) SO_TIMEOUT applies to each read independently: a peer trickling a response one byte at a time during a deadline-bounded poll reset its clock with every byte, stretching status/header/body parsing arbitrarily past the poll's absolute deadline. Every blocking read of the response now goes through beforeBlockingRead(), which re-caps SO_TIMEOUT to what is left of the deadline (skipping the syscall outside poll mode, and while buffered data makes the next read non-blocking). Unit test: a peer trickling one byte per 300 ms against a 500 ms poll is cut off by the shared 1.5 s budget instead of taking ~12 s for the ~40-byte response. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46679c7fda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…line (Codex P2) The fixed 1 s fault headroom sat ON TOP of the requested wait, so waitFor(Duration.ofMillis(1)) could block for about a second. The transit slack for the server's "nothing yet" op-timeout fault is now carved out of the INSIDE of the budget instead: the Receive asks the server to answer at budget - min(1s, budget/2), and the socket cuts at the budget itself — pollTimeout(budget) adds nothing on top. A wait too short for any network round trip (< 100 ms) is waited out locally, without touching the wire. Tests: a 2 s poll sends OperationTimeout PT1S and treats the fault arriving within the budget as "nothing yet" (cursor stays fully usable); a dead peer fails a 200 ms wait at ~200 ms with the server asked to answer in PT0.1S; a sub-round-trip wait leaves the wire untouched and the process readable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daf662114b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A bounded poll that discovered completion sent the terminate Signal under the full inactivity timeout: a server reporting Done promptly but stalling on the Signal answer could hold waitFor(Duration) far past its advertised hard deadline. The completion cleanup of a bounded poll now runs through finishBounded(): the Signal round trip gets the poll's remaining budget (pollTimeout + the shortened envelope OperationTimeout); a Signal answer that does not arrive in time is abandoned — the connection is dropped so the late response cannot desync a later request — and NEVER reported, because the command has completed and its exit code is known. A budget too small for any round trip skips the Signal outright, leaving the healthy connection untouched (the server reaps the completed command's state with the shell). The plain nextChunk()/close() paths keep their full-timeout Signal. Tests: a Signal stalling 3 s past a 1 s poll still reports completion at ~1 s with the exit code; a 20 ms-budget completion reports without touching the wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58623c4d1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nup best-effort (Codex P2s) - waitFor(Duration) could return false when the final Done-carrying chunk consumed the last of the deadline: the wrapper only learns completion from the NEXT cursor call, which the expired loop never made. After the loop, a local-state probe (cursor.exitCode()) now detects the absorbed completion and finalizes it — the follow-up advance runs on a 1 ms budget, so it skips the wire Signal and cannot wait. - finishBounded() swallowed only SocketTimeoutException: a WSMan fault or a connection reset answering the completion Signal escaped and hid the known exit code. Every failure of that cleanup is now best-effort — a fault (a complete, in-sync exchange) is ignored and the connection kept; anything else drops the connection so a late response cannot desync a later request. The completion always wins. Tests: a Done response landing ~80 ms before a 600 ms deadline reports true with the exit code (and no Signal, since none fits); a fault answering the completion Signal is ignored and the connection stays usable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
New doc-site page cli.md — a manual page for the standalone jar: synopsis, subcommands, the full option table, password handling, Kerberos KDC/realm configuration and inference, streaming output behavior, timeout semantics, exit codes, and examples. Published at https://metricshub.org/winrm-java/cli.html and added to the site menu. Everything that duplicated it now points there instead: --help shrinks to usage + options + the manual link; the per-topic CLI sections of wql.md, commands.md and installation.md become one-line pointers; the exit-code table moves out of timeouts-and-errors.md; the README CLI section keeps the quick-start commands and links to the manual for the rest. Also document in ChunkDecoder's Javadoc why the class exists at all: it is a thin push-style convenience over the JDK's own CharsetDecoder — the JDK's ready-made incremental decoders (InputStreamReader/StreamDecoder) only fit pull-based streams, and WSMan output chunks are pushed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7efc72dc83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…st-effort (Codex P2s) - The streaming (inactivity) socket mode relied on SO_TIMEOUT alone, which restarts on every byte: a peer trickling an endless incomplete response could hold a WqlCursor.next()/CommandCursor.next() forever despite the documented per-round-trip bound. inactivityTimeout mode now arms an absolute per-leg deadline at the start of every post() (reconnect included), enforced by the existing per-read re-caps — one whole response must arrive within the inactivity timeout. - The ordinary nextChunk() EOF path still propagated faults/resets from the terminate Signal of an ALREADY-COMPLETED command, failing a command whose exit code was known. finish() now routes completed commands through the shared best-effort terminateCompleted() (fault → ignored, connection kept; anything else → connection dropped), while an early close of a still-running command keeps its error reporting — there the Signal is what actually stops the command. Tests: a trickling peer is cut off at the inactivity timeout in streaming mode; a fault answering the completion Signal on the plain next() path reports EOF + exit code and leaves the connection usable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #111 — phase 2 of the API modernization, on top of the fluent builders from #131.
What's new
The choice between "materialize everything" and "stream" is a different terminal method on the same operation builder — everything upstream (auth, TLS, namespace, options) is shared:
execute()→WqlResultstream()→ closeableStream<WqlRow>execute()→CommandResultstart()→RemoteProcess(Process-like, closeable)Plus the callback middle ground:
.onStdout(chunk -> ...)/.onStderr(chunk -> ...)tail the output live whileexecute()keeps its blocking contract and completeCommandResult.Semantics (as specified in #111)
Pullis issued only as the stream advances; command chunks are delivered perReceiveresponse. Memory is bounded by the active page (pageSize(int)) / the unread channel.Release(new envelope — the client never sent one before, helping server-side operation quotas); exhaustion releases the connection on its own. Closing aRemoteProcessearly sends the terminateSignalthat actually stops the remote command; completion signals on its own.OperationTimeoutheader, its fault code, and the socket read timeout), not an overall deadline.RemoteProcess.waitFor(Duration)provides a hard deadline. Documented in the Javadoc, README and the doc site.ResultSet-style). The internal lock became aSemaphoreso a handle can be legitimately closed from a different thread than the one that opened it.RemoteProcessreaders and the callbacks decode with a statefulCharsetDecoder(ChunkDecoder) that carries partial-character state between chunks — a multibyte character split acrossStreamelements orReceiveresponses decodes correctly (unit-tested at every split position; the blocking path keeps its accumulate-then-decode behavior byte-for-byte).Implementation
WsmanClientnow exposes stream-first primitives (WqlEnumeration,RemoteCommand); the blockingwql()/executeCommand()are implemented as "drain the stream" over them, so the two paths cannot drift apart. The blocking paths' wire behavior is unchanged (same request sequences, no Release on error paths, finally-Signal semantics preserved, cancellation checks kept before every side-effectful step).WindowsRemoteExecutorgainedstreamWql()/startCommand()default methods (throwingUnsupportedOperationException, like the namespace-awareexecuteWqlprecedent) returning the new publicWqlCursor/CommandCursorSPI types;LightWinRMServiceimplements them.Tests
18 new tests (131 total, all green with
mvn verify site): multi-page enumeration laziness (asserted on the fake server's decrypted request log), early close →Releasewith the right context, exhaustion → noRelease+ connection reuse, op-timeout fault → inactivityWinRMTimeoutException(WQL and command), incremental stdout before command completion, interleaved channel ordering, split multibyte characters acrossReceiveresponses (readers and callbacks), early close → terminateSignal+ connection reuse,waitFor(Duration)expiry, callback chunk delivery with complete final result, SPI defaults rejection, andChunkDecoderunits (every split position of 2/3/4-byte UTF-8, malformed input parity withnew String).🤖 Generated with Claude Code