Add the fluent WinRMClient API: builders, typed results, unchecked exceptions - #133
Conversation
…ceptions) The new front door of the library (issue #131): WinRMClient.builder(host) creates a reusable, AutoCloseable client — one authentication, any number of WQL queries and commands over the same connection. Per-operation builders end in execute() and return typed results (WqlResult/WqlRow with case-insensitive lookup, CommandResult), with Duration timeouts throughout. Failures surface through a new unchecked hierarchy (WinRMClientException + Authentication/Fault/Timeout/WqlSyntax subtypes); WinRMFaultException carries the WSManFault code, reason, and provider detail as fields. The legacy static helpers are unchanged. Also closes #86: pageSize(int) and pullTimeout(Duration) on the WQL builder plumb MaxElements and MaxTime down to the WSMan envelopes (defaults unchanged: OptimizeEnumeration + MaxElements=32000). Supporting changes to the light internals: - WsmanClient throws the typed WinRMFaultException / WinRMAuthenticationException (messages byte-identical to before). - The WSMan OperationTimeout header and the socket read timeout now follow each operation's own timeout instead of the creation timeout (identical values through the legacy API; the fluent API can override per operation). - LightWinRMService/WinRMExecutorFactory gain a TLS-override overload (SSLContext or trust-all) backing the per-client trustAllCertificates() and sslContext(...) builder options. - ShellFileCopy.copyLocalFileToRemoteFile(...) uploads to an explicit remote path (digest-verified, skip-if-identical, directory created), backing client.uploadFile(...). Tests: builder validation, WqlRow semantics, and e2e coverage of the new API against FakeWsmanServer (typed results, wire effect of pageSize / pullTimeout / namespace / timeout, charset detection caching, typed fault/auth/timeout mapping); FakeWsmanServer gains enqueueDelayed() for a deterministic timeout test. README, site index, and CHANGELOG updated. Closes #131 Closes #86 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a77af5700d
ℹ️ 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".
…idation - P1: a worker whose wall-clock timeout fired while its operation was still queued behind another one on the serial connection no longer executes the operation after the caller was told it timed out. WsmanClient acquires the operation lock interruptibly and re-checks the interrupt flag right after acquisition (Utils.execute cancels the worker with an interrupt on timeout). Regression test verified to fail without the fix: the abandoned command reached the wire. - P2: uploadFile destinations must be drive-rooted (C:\...) or UNC (\server\share\...); relative and drive-relative (C:x.ps1) paths are rejected instead of resolving against the remote shell's current directory. - P2: Duration options now require at least one millisecond; a positive sub-millisecond duration used to truncate to 0 and be rejected later (timeouts) or silently ignored (pullTimeout). 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: f4df143823
ℹ️ 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".
- P1: a worker blocked in a socket read does not observe the cancellation interrupt, so a first command whose timeout fired during shell creation went on to START the command once the late Create response arrived. WsmanClient now re-checks the interrupt flag between protocol steps (before Command, and before each WQL Pull). Regression test: the Create response arrives after the caller's timeout, the abandoned worker must not start the command, and the client stays usable for the next command on the now-existing shell. - P1: restored the credentials(...) Javadoc that a formatter pass had mangled (parameter text split, stray asterisks, @return concatenated into the @param text). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
When a command times out while this socket read is blocked, Future.cancel(true) interrupts the worker but the read does not observe that interrupt. After a late non-final response—or a WSMan operation-timeout fault—the loop sends another Receive without calling checkNotCancelled(), so the abandoned worker can continue polling and holding operationLock until the remote command ends, causing subsequent operations on the reusable client to queue or time out. Check for cancellation before every Receive iteration.
ℹ️ 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".
… timeout - P1: the Receive loop now re-checks the cancellation flag before every iteration, so a worker whose timeout fired mid-command stops polling when a late partial response (or op-timeout fault) arrives, terminates the command via the finally-block Signal, and releases the serial connection instead of holding it until the remote command ends. Regression test: partial Receive response arriving after the timeout — exactly one Receive and one Signal for the abandoned command, and the next command runs normally. - P2: HttpTransport.operationTimeout(...) now drives the connect timeout too, not just the read timeout, so a (re)connection made on behalf of an operation is bounded by that operation's own timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The review-body P1 (abandoned worker kept re-issuing Receive after its timeout) is fixed in 29b10e2: the Receive loop re-checks the cancellation flag before every iteration, terminates the command via the finally-block Signal, and releases the connection — with an e2e regression test (late partial response after the timeout → exactly one Receive + one Signal for the abandoned command, next command runs normally). @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 "File Transfers" documentation page covering the full mechanics of copying files through the WinRM channel: the two entry points (upload() on the command builder / uploadFile() to an explicit path, plus the legacy localFileToCopyList), the transfer directory (<windir>\Temp\SEN_ShareFor_<CLIENT>$) and how it is discovered, content-addressed remote names and MAX_PATH truncation, the transfer pipeline (skip-if-identical digest probe, chunked base64 echo legs, certutil decode into a staging file, verified publish), the temporary .part/.part.b64 files, the 30-day purge, case-insensitive command-line substitution and the CMD.EXE /C wrap, and the file-name/size/quota constraints. Linked from the site menu, the overview, Remote Commands (whose inline explanation is condensed into a pointer), and the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again (docs-only commit: new File Transfers page) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb5cf81b6b
ℹ️ 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".
…epair-failure semantics 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: 104ac70162
ℹ️ 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".
…n CIMV2 - P1: a cached command shell reaped by the server between commands (e.g. IdleTimeout on a long-lived client) left the client permanently broken: every command faulted shell-not-found against the stale selector. WsmanClient now catches the shell-not-found fault on Command creation (safe: the command was rejected before it could run), clears the cached ID, recreates the shell, and retries once; a shell-not-found on the terminate Signal also drops the cached ID so the next command starts clean. - P1: the internal housekeeping WQL queries now explicitly target ROOT\CIMV2 via the new WmiHelper.executeWqlInCimv2 (with a fallback to the executor default when the extended executeWql is unsupported): encoding detection (Codex's finding) and Windows-directory discovery for file transfers (same bug, found by inspection) both faulted when the client was built with a custom default namespace. e2e regression tests for both, CHANGELOG updated. 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: 80d252e104
ℹ️ 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".
- wql.md, commands.md, authentication.md, tls.md, timeouts-and-errors.md now document the fluent WinRMClient API exclusively: builders, typed results (WqlResult/WqlRow with case-insensitive lookup, CommandResult), Duration timeouts as a single wall-clock deadline, the unchecked exception hierarchy with programmatic fault fields, per-client TLS (trustAllCertificates / sslContext with a worked trust-store example), ordered authentication fallback, and the pageSize/pullTimeout enumeration knobs. CLI sections unchanged. - New legacy.md summarizes the static one-shot helpers (signatures, results, checked exceptions, behavior notes) for reference; linked from the Reference menu and the overview. - migrating-from-1x.md gains a "Moving to the fluent API" section: before/after examples for WQL and commands, a 1.x-to-fluent option mapping table, and the checked-to-unchecked exception mapping (with the message-compatibility guarantee). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…(Codex P2) The first command on a connection pins the shell's working directory; every (re)creation — including the transparent recreation after the server reaped the shell — now reuses it, instead of the current request's (usually null) value. Regression test asserts the recreated shell carries the original WorkingDirectory. 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: e21c2ea173
ℹ️ 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 username with a leading or trailing backslash (empty domain or user part) now fails fast with a clear IllegalArgumentException at the setter, instead of an ArrayIndexOutOfBoundsException from the endpoint parser at build(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
Implements the phase-1 API modernization designed in #131: a fluent, reusable
WinRMClientas the new front door of the library — purely additive, the legacy static helpers are untouched.The new API
WqlResult(iterable,columns()in query order) /WqlRow(case-insensitive property lookup, like WMI) /CommandResult(exitCode(),elapsed()asDuration).WinRMClientExceptionwithWinRMAuthenticationException,WinRMFaultException(WSManFault code, reason, and provider detail as fields — no morecontains()matching on messages),WinRMTimeoutException,WqlSyntaxException. Message texts stay byte-identical to the legacy API.trustAllCertificates()/sslContext(SSLContext)override the globalorg.metricshub.winrm.tls.insecuresystem property per instance.client.uploadFile(local, remotePath)— digest-verified transfer through the WinRM channel to an explicit destination, directory created when needed (newShellFileCopy.copyLocalFileToRemoteFile(...)seam).Light-internals changes backing it
Envelopes/WsmanClientparametrizeMaxElementsand emitMaxTimeon Pull when requested (defaults unchanged and pinned by tests:OptimizeEnumeration+MaxElements=32000, noMaxTime).WsmanClientnow throws the typedWinRMFaultException/WinRMAuthenticationExceptioninstead of bareIllegalStateExceptionon fault/auth paths (same messages).OperationTimeoutheader and the socket read timeout now follow each operation's own timeout instead of the executor's creation timeout. Through the legacy API both values were always identical, so this is behavior-preserving there; it makes the fluent per-operationtimeout(...)fully effective on the wire.WindowsRemoteExecutorgains a defaultexecuteWql(namespace, wql, timeout, maxElements, pullTimeout)(throwsUnsupportedOperationExceptionunless overridden — only the light backend implements it), keeping the interface backward compatible.Stream-first note (per #131 / #111)
The blocking
execute()paths go through seams (WsmanClient.wqlPull loop,receiveLoop) that phase 2 (#111) will refactor into iterator/callback shape; nothing in the public API changes for that —stream()/start()become additional terminal methods.Tests & docs
WinRMClientBuilderTest: builder/request validation, defaults,WqlRowsemantics, offlinebuild().WinRMClientTest: e2e againstFakeWsmanServer(real NTLM handshake + message encryption) — typed results, wire effect ofpageSize/pullTimeout/namespace/timeout, charset-detection caching and shell reuse across commands, typed fault/auth mapping (exact legacy messages), deterministic timeout via a newenqueueDelayed(...)on the fake server, closed-client behavior.ShellFileCopyTest: explicit-destination upload (round-trip of echoed bytes, MKDIR of the destination directory, skip-if-identical, path validation).mvn clean verify sitegreen — 109 tests, checkstyle/PMD/SpotBugs reports clean.Closes #131
Closes #86
🤖 Generated with Claude Code