[ideaship]

Two commits:

• Fix the manageable state choice. --provision-state offered managable. Ironic's state is manageable, so the misspelled choice could never match a node, and argparse rejected the correct spelling. Listing the manageable nodes through this option was impossible.
• Add --details. Without the flag, the output is unchanged. With it, osism baremetal list appends five columns:
  • State Since (UTC): when the node entered its current provision state (Ironic's provision_updated_at). After a round of redeploys, this shows which nodes have been redeployed and which still run their old installation.
  • Target State: target_provision_state, i.e. where a node in a transitional state such as wait call-back is heading.
  • Automated Clean: the node's automated_clean. Ironic's null means the node follows the conductor's setting, so it is shown as default rather than False.
  • Maintenance Reason: the reason set by hand, or Ironic's fault (e.g. power failure) when Ironic put the node into maintenance itself.
  • Last Error: last_error, flattened to one line and cut to 80 characters.

openstacksdk does not map provision_updated_at

Ironic returns the field, but openstacksdk's baremetal Node resource has no attribute for it, so the SDK discards it while parsing the response. The new osism/utils/ironic.py therefore lists nodes through a Node subclass, DetailedNode, that declares the field. It uses Resource.list, the public method behind conn.baremetal.nodes(), so the request and microversion negotiation are the same. There is no raw API call.

The subclass also has to repeat Node's uuid alternate-id declaration. openstacksdk looks the alternate id up only in the class's own __dict__, not in its parent classes, so without that line a subclass reports None as every node's id. A test covers this.

Both points look like upstream openstacksdk issues. Once a fixed openstacksdk is in our pin, the subclass can be removed.

Testing

• New unit tests for both commits. pytest, black, flake8 and mypy pass locally.
• Not yet run against a live Ironic.

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've found 3 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="osism/commands/baremetal.py" line_range="254-257" />
<code_context>
+                    row += [
+                        _format_state_since(b["provision_updated_at"]),
+                        b["target_provision_state"] or "",
+                        _format_automated_clean(b["automated_clean"]),
+                        _format_maintenance_reason(b),
+                        _shorten_last_error(b["last_error"]),
+                    ]
                 result.append(row)

</code_context>
<issue_to_address>
**Node details can inject terminal controls**

When an attacker can cause Ironic to return terminal control characters in a node detail string, the details row passes these strings to `tabulate` unchanged (`_shorten_last_error` only collapses whitespace), so ANSI/OSC sequences reach the operator's terminal and can forge output or trigger terminal actions.

Escape or remove terminal control characters from node-provided strings before printing.
</issue_to_address>

### Comment 2
<location path="osism/utils/ironic.py" line_range="38" />
<code_context>
+    This goes through the same request and microversion negotiation as the
+    SDK's own proxy method; only the resource class differs.
+    """
+    return DetailedNode.list(conn.baremetal, details=True, **query)
</code_context>
<issue_to_address>
**Detailed columns stay empty**

When the user runs `osism baremetal list --details`, `DetailedNode.list` bypasses the baremetal proxy’s detail-list routing, so Ironic returns summary nodes or rejects the unsupported parameter, leaving detail columns empty or making the command fail.

Route the request through the baremetal proxy’s detail-list method.
</issue_to_address>

### Comment 3
<location path="tests/unit/commands/test_baremetal.py" line_range="635-637" />
<code_context>
+    _run_list_details([], [node])
+
+    out = capsys.readouterr().out
+    assert "first line second line" in out
+    assert "x" * 200 not in out
+    assert "..." in out
+
+
</code_context>
<issue_to_address>
**Wrong error truncation width passes**

When the formatter truncates long errors at a width other than 80, `test_list_details_last_error_flattened_and_shortened` checks the prefix, absence of the full 200-character suffix, and presence of `...`; truncating to 25 characters instead of 80 still satisfies all three assertions, so the test passes while errors are cut to the wrong length.

Assert that the displayed error contains the expected 77-character prefix followed by `...` (or test the formatter directly for an exact 80-character result).
</issue_to_address>

Sourcery assessment

Approval pending. 3 findings to address first.

Blocking findings: osism/commands/baremetal.py:257, osism/utils/ironic.py:38, tests/unit/commands/test_baremetal.py:637

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook