Repository navigation
ipmicli: repeated -p/-k message, terminal detection on Java 22-24, --no-escape on a terminal, 'power on' hint, owner in sensors and fru output (#218) - #232
Merged
Conversation
…2-24, refuse --no-escape on a terminal, name 'power on' in the cycle refusal, and print the owner of sensors and FRUs (#218) - CliArguments: a password or BMC key option given twice, in either of its forms, fails with '<option> is given twice'; the mutual-exclusion message is kept for -p with -pf and -k with -kf. - IpmiCli.main() takes System.console() for a terminal only when Console.isTerminal(), called by reflection, says so (Java 22 to 24 return a console for redirected streams); readConsolePassword() refuses such a console with the same 'no interactive console' message. - sol --no-escape is a usage error on a terminal, where no keystroke would close the console; the banner no longer promises that the end of the input closes it. - power cycle of a system that is off says "use 'power on'" instead of the PowerUp constant. - sensors --json adds ownerId (encoded like the low byte of a SEL generator ID), ownerLun and ownerChannel; fru --json adds address, lun and channel, and the text header and problem lines name the controller and LUN of a FRU not held by the BMC at LUN 0. Checked with the standalone jar on JDK 21 and 25: a redirected run refuses to prompt, and the reflective isTerminal() returns true on a real terminal of Java 25. The Java 22 to 24 case itself is traced from the JDK API (no such JDK installed; Java 25 returns a null console for redirected streams even with -Djdk.console=jdk.internal.le). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8496e288b7
ℹ️ 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".
…view) FruReader reads a locator at 20h as a FRU of the BMC whatever its channel (a Supermicro BMC locates its own FRU 0 at 20h on channel 2): the label depends on the address and LUN only, and names the channel for another controller. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bertysentry
deleted the
218-ipmicli-wrong-message-for-a-repeated--p-k-redirected-runs-taken-for-a-terminal-on-java-22-24-false---no-escape-banner-powerup-hint-no-owner-in-sensors-and-fru-output
branch
October 25, 2026 20:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #218: the five CLI-layer defects, no library API change.
What changes
-p/-kreported as repeated.CliArgumentsrecords the long name of the secret option given (-pand--passwordare the same) and refuses a repetition with--password is given twice(--bmc-key is given twice);-pwith-pfand-kwith-kfkeep the mutually exclusive message. Other repeated options still take their last value.IpmiCli.main()passesisTerminal(System.console()), which callsConsole.isTerminal()by reflection when it exists (Java 22+) and takes a non-null console for a terminal otherwise (Java 8 to 21).readConsolePassword()refuses a non-terminal console with the same no interactive console message. A redirected run therefore never prompts, andsoltakes the redirected-input path (bytes forwarded as they come) instead of raw or line mode.--no-escapeon a terminal is refused (exit 64):--no-escape needs a redirected input: on a terminal, ~. is the only way out (use --escape to pick another character). The banner's "the end of the input closes it" branch is gone with it: on a terminal, an escape sequence is always set.power cycleof a system that is off printsipmicli: the system is off: a power cycle would power it up on some BMCs, use 'power on'(exit 70) instead of the client message namingPowerUp.sensors --jsonaddsownerId(the 7-bit address shifted left, bit 0 set for a system software ID, the encoding of the low byte of the SELgeneratorId),ownerLunandownerChannel, so a script joinssel --jsonasSelEntry.isFrom()does.fru --jsonaddsaddress(8-bit, 32 for the BMC),lunandchannel; the text header and the problem lines name them when the FRU is not held by the BMC at LUN 0 (FRU 5 at 2Ch channel 6: ...,FRU 5 at 20h LUN 1: ...).Docs:
cli.md(options,sensors,fru,seljoin, SOL, exit codes) and the Command-line client section ofupgrading.md.Tests
CliArgumentsTest.usageErrorsand the wipe test cover the repeated options;IpmiCliTestadds the owner fields of sensors on the BMC, at LUN 1, on a satellite (2Ch, channel 6) and in the BIOS (B1h), three FRUs numbered 5 on different controllers and LUNs, the--no-escaperefusal withterminal=true, andisTerminal()(false for a null console and for the console of the forked test JVM, whose streams are pipes on every JDK). The golden JSON files gain the new fields.mvn clean verify sitepasses with no new checkstyle, PMD, CPD or SpotBugs finding.Live checks
With the standalone jar: on JDK 21 and JDK 25,
echo secret | ipmicli ... chassis > out.txtfails with no interactive console to ask the password (exit 64); on JDK 25 the reflectiveConsole.isTerminal()returns true on a real terminal (ProxyingConsole). The Java 22 to 24 case itself (a non-null console whoseisTerminal()is false) is traced from the JDK API: no such JDK is installed, and Java 25 returns a null console for redirected streams even with-Djdk.console=jdk.internal.le(JDK-8351435), so the issue's JDK 21 reproduction with that provider still prompts there, asisTerminal()does not exist on 21. The repeated-p/-kmessages were checked with the jar; the FRU labels of other controllers are by the specification, as no lab BMC has such a FRU (#193).🤖 Generated with Claude Code