Migrate power control handling to roxyd

[zmrdltl]

Background

• roxyd already routes NodePowerRequest into handlers::power::handle, but src/bin/roxyd/handlers/power.rs is still scaffolding only
• The legacy roxy path already implements immediate reboot and immediate power-off plus graceful reboot and graceful power-off in src/root/task.rs
• This issue intentionally covers the full power family rather than only a restart-shaped subset, and the current roxyd control path already routes legacy flat reboot and shutdown requests through node_power
• In review-protocol, NodePowerResponse::Initiated means the node accepted the power command and may become unreachable shortly after the response is received
• Because roxyd sends the response only after the power handler returns in the upstream review_protocol::request::handle() flow, an inline Linux reboot(2) or poweroff(2) path cannot satisfy the Initiated reply contract
• The current dispatch path writes the power response after handlers::power::handle returns, so the immediate reboot and shutdown path requires a small roxyd-local dispatch-side change in addition to the handler work
• The repository CI currently runs on GitHub-hosted ubuntu-latest and macos-latest, so normal CI coverage must not reboot or shut down the runner
• This issue is for migration, not behavior redesign

Goal

• Implement the full power family directly in the roxyd power path
• Preserve current legacy behavior and platform boundaries while adapting to NodePowerRequest, NodePowerResponse, and the reply timing contract
• Close the first-pass execution path enough that implementation can start without reopening the design during this issue

Scope

Immediate Reboot And Shutdown

• Implement NodePowerRequest::Reboot and NodePowerRequest::Shutdown handling in the roxyd power path
• NodePowerRequest::Shutdown maps to the legacy immediate PowerOff intent
• Immediate Reboot and Shutdown remain Linux-only as in the legacy path
• The immediate path must use an implementation that lets roxyd write NodePowerResponse::Initiated successfully before reboot or power-off begins
• For the first pass, use an in-process pending-operation approach for immediate reboot and power-off
• The immediate power path must prepare a pending reboot or shutdown operation without executing it immediately
• The pending operation must wait for an explicit release signal before calling nix::sys::reboot::reboot(...)
• roxyd must write NodePowerResponse::Initiated on the current request stream before releasing the pending operation
• roxyd must release the pending operation only after the success response write completes successfully
• If writing the success response fails, the pending operation must be dropped or cancelled without rebooting or powering off
• Add a small roxyd-local power-aware dispatch path for immediate NodePowerRequest::Reboot and NodePowerRequest::Shutdown so roxyd owns the response-write ordering
• Preserve the existing flat reboot and shutdown compatibility requests through the same ordering-safe path
• This issue does not require a hidden roxyd one-shot mode, hidden internal CLI surface, parent/child readiness protocol, or child-process release protocol
• Do not call nix::sys::reboot::reboot(...) inline from the normal request handler before the success response is written
• Do not replace the immediate path with shelling out to reboot or poweroff
• Unsupported non-Linux immediate requests and preparation failures detected before the success response return Err("invalid command".to_string())

Graceful Reboot And Shutdown

• Implement NodePowerRequest::GracefulReboot and NodePowerRequest::GracefulShutdown in src/bin/roxyd/handlers/power.rs
• NodePowerRequest::GracefulShutdown maps to the legacy GracefulPowerOff intent
• GracefulReboot and GracefulShutdown remain aligned with the current legacy platform behavior
• Graceful start failures return Err("fail".to_string())

Response And Compatibility

• Return NodePowerResponse::Initiated when the power request has been accepted and the execution path is ready to proceed
• Handle power operations directly in roxyd without using run_roxy() or the legacy subprocess path
• Keep the existing flat reboot and shutdown requests working through the same ordering-safe power path
• Preserve response-write ordering for immediate reboot and shutdown within roxyd without requiring a review-protocol change
• Do not introduce a new client-visible failure contract for errors that happen after NodePowerResponse::Initiated has already been returned
• Keep any additional intentional behavior change out of this issue

Testing

• If needed for safe testing, allow a minimal helper extraction or other small indirection at the reboot and shutdown operation boundary
• Add or update tests for power handler/helper behavior and dispatch behavior
• Normal test and CI runs must not reboot or shut down the real test host or a GitHub Actions runner

Acceptance Criteria

• Immediate: On Linux, NodePowerRequest::Reboot preserves the current legacy immediate reboot intent and writes NodePowerResponse::Initiated successfully before reboot begins
• Immediate: On Linux, NodePowerRequest::Shutdown preserves the current legacy immediate power-off intent and writes NodePowerResponse::Initiated successfully before power-off begins
• Immediate: On non-Linux platforms, immediate Reboot and Shutdown remain unsupported and return the error string invalid command
• Immediate: reboot and shutdown do not execute the legacy inline nix::sys::reboot::reboot(...) call inside the normal request handler before the success response is written and do not replace the immediate path with shelling out to reboot or poweroff
• Immediate: preparation failures detected before the success response return invalid command
• Immediate: the pending operation does not begin reboot or power-off before the current request stream has written NodePowerResponse::Initiated successfully
• Immediate: if writing the success response fails, the pending operation is dropped or cancelled without rebooting or powering off
• Graceful: NodePowerRequest::GracefulReboot initiates a graceful reboot and returns NodePowerResponse::Initiated
• Graceful: NodePowerRequest::GracefulShutdown initiates the legacy graceful power-off behavior and returns NodePowerResponse::Initiated
• Graceful: start failures return fail rather than unimplemented!() or panic
• Compatibility: the canonical grouped node_power path and the existing flat reboot and shutdown compatibility paths preserve the expected success and failure behavior
• Compatibility: response-write ordering for immediate reboot and shutdown is preserved within roxyd without requiring a review-protocol change
• Compatibility: the roxyd power path does not invoke run_roxy() or the legacy roxy binary
• Compatibility: this issue does not add a new client-visible failure response for errors that happen after NodePowerResponse::Initiated has already been returned
• Testing: tests cover direct power handler behavior, grouped node dispatch, and the flat reboot and shutdown compatibility paths without rebooting or shutting down the real test host during normal runs
• Testing: normal CI test runs do not reboot or shut down a GitHub Actions runner

Tasks

Immediate Reboot And Shutdown

• Implement NodePowerRequest::Reboot and NodePowerRequest::Shutdown handling in the roxyd power path
• Map NodePowerRequest::Shutdown to the legacy PowerOff intent
• Add an in-process pending-operation path for immediate reboot and power-off
• Add an explicit release step that runs the pending operation only after NodePowerResponse::Initiated is written successfully
• Add the roxyd-local power-aware dispatch path for immediate NodePowerRequest::Reboot and NodePowerRequest::Shutdown
• Preserve the flat reboot and shutdown compatibility paths through the same ordering-safe path
• Implement the immediate reboot and power-off path so roxyd writes NodePowerResponse::Initiated successfully before reboot or power-off begins, without calling nix::sys::reboot::reboot(...) inline from the normal request handler
• Map unsupported non-Linux immediate requests and preparation failures detected before the success response to Err("invalid command".to_string())

Graceful Reboot And Shutdown

• Implement NodePowerRequest::GracefulReboot and NodePowerRequest::GracefulShutdown in src/bin/roxyd/handlers/power.rs
• Map NodePowerRequest::GracefulShutdown to the legacy GracefulPowerOff intent
• Map GracefulReboot and GracefulShutdown start failures to Err("fail".to_string())

Testing And Compatibility

• If needed for safe testing, extract a minimal power-operation boundary that allows success and failure paths to be tested without rebooting or shutting down the real host during normal test runs
• Add focused power handler/helper tests for immediate and graceful power success and failure paths
• Add a test that verifies a success-response write failure does not allow reboot or power-off to begin
• Update the roxyd live dispatch tests for the canonical grouped power path and the existing flat reboot and shutdown compatibility requests so they verify the expected success or failure behavior instead of the current unimplemented panic path
• Verify that the implementation preserves the current platform boundaries and does not depend on the legacy subprocess wrapper path

Non Goals

• Removing the legacy roxy binary or legacy power code
• Migrating other handlers under Migrate roxy functionality into roxyd Node handlers #609
• Behavioral cleanup, API normalization, or client-visible semantic changes around power handling
• Expanding immediate reboot or shutdown support beyond the current legacy platform boundaries
• Changing the review-protocol power reply contract or adding a new client-visible failure channel after NodePowerResponse::Initiated
• Requiring live reboot or shutdown coverage in normal test runs or CI
• Introducing a hidden roxyd one-shot CLI or subprocess protocol for the first-pass migration

[zmrdltl]

Marking this issue as blocked until the maintainer and Kone review it and approve implementation.

[octoaide]

@sae-bom — gentle ping for review/approval on issue #613 (Migrate power control handling to roxyd). The issue author marked this blocked pending your and Kone's approval before implementation. The issue is otherwise fully specified and actionable. Summary of the plan I intend to implement once you (and Kone) approve:

• Implement NodePowerRequest::Reboot and ::Shutdown in src/bin/roxyd/handlers/power.rs, mapping Shutdown to the legacy PowerOff intent. Keep immediate-path support Linux-only and return Err("invalid command") on non-Linux or early-start failures.
• Add an internal one-shot roxyd execution mode (hidden subcommand/flag) that: skips normal daemon startup and manager connection, reports readiness, waits for an explicit release from the request handler, and only then performs the reboot/poweroff. If response write fails, it exits without rebooting.
• Ensure the handler writes NodePowerResponse::Initiated before releasing the waiting one-shot path. If required, add a minimal roxyd-local dispatch change to guarantee response-write ordering for immediate requests (keeps compatibility and doesn't change review-protocol).
• Implement graceful reboot/shutdown paths mapped to legacy graceful intents; start failures return Err("fail").
• Add handler- and dispatch-level tests that exercise success/failure and verify that CI/test runs do not actually reboot/poweroff (extract a small test seam/mock at the reboot boundary if needed).

Please confirm one of the following so I can proceed:

1. Approve the plan and I will open a PR implementing the above (I can also include requested reviewers on the PR).
2. Approve implementation but request specific constraints (e.g. preferred internal subcommand name or conventions for hidden flags).
3. If you prefer to handle the change yourself, please assign someone and/or indicate desired reviewer(s.

If you want me to proceed to implement immediately, please reply with a short approval ("go ahead" / 👍) and any small preferences about the internal CLI surface. Thanks!

[octoaide]

@sae-bom — gentle ping for review/approval on issue #613 (Migrate power control handling to roxyd). The issue author marked this blocked pending your approval; the issue itself is fully specified and actionable. In my previous comment I summarized the concrete plan I intend to implement once you approve: implement immediate and graceful power paths in src/bin/roxyd/handlers/power.rs, add a hidden one-shot internal roxyd execution mode to preserve the NodePowerResponse::Initiated reply contract (report readiness, wait for release, only reboot/poweroff after the response is written), keep Linux-only immediate behavior and test safety in CI, and add focused tests for success/failure without rebooting CI hosts. Please confirm one of the following so I can proceed: 1) Approve the plan and I will open a PR implementing the change (I can add requested reviewers). 2) Approve but request small constraints (e.g., a preferred internal subcommand name or conventions for hidden flags). 3) If you prefer to handle the change yourselves, please assign someone and/or indicate desired reviewer(s). If you want me to proceed to implement immediately, please reply with a short approval ("go ahead" / 👍) and any small preferences about the internal CLI surface. Thanks!

[sae-bom]

I understand the advantage of the proposed one-shot execution path. It keeps immediate reboot/shutdown separated from the roxyd daemon's async shutdown and cancellation model, and it gives the power action a clear external execution boundary instead of turning it into another daemon-owned background task. That seems directionally good, especially if we later introduce a repository-wide cooperative shutdown / cancellation-safety model.

That said, since this issue is targeting a first-pass migration, I wonder whether this is the right amount of mechanism to introduce here. Immediate reboot(2) / poweroff(2) is also not the normal operational path we should encourage in our system; in normal cases, graceful reboot/shutdown should be preferred. Because of that, I am not sure the first migration issue needs to introduce a hidden roxyd one-shot mode, parent/child readiness reporting, and an internal release protocol.

Would a smaller in-process pending operation be enough for this issue? The key requirement seems to be ordering, not necessarily a separate process. One possible shape is:

• the immediate power handler prepares a pending reboot/shutdown operation instead of executing it immediately
• that pending operation owns a release channel and waits for an explicit release signal before calling reboot(2) / poweroff(2)
• the roxyd request side writes NodePowerResponse::Initiated first
• only after the response write succeeds does roxyd send the release signal
• if the response write fails, the release handle is dropped and the pending operation exits without rebooting or powering off

To make that work, roxyd would likely need a small power-aware dispatch path instead of relying entirely on the upstream review_protocol::request::handle() flow for this case. The current upstream flow calls the handler first and writes the response after the handler returns, so the handler cannot know whether the response write actually succeeded. A local roxyd dispatch path for NodePower, and the flat reboot / shutdown compatibility requests if needed, could preserve the same wire contract while taking ownership of the ordering: prepare pending action, write response, then release.

This would still preserve the important response-ordering contract while avoiding the hidden one-shot CLI and child-process protocol in the first pass. Did you already consider this kind of lower-cost approach before choosing the one-shot process direction?

[octoaide]

@sae-bom — I previously summarized a concrete plan that adds a hidden roxyd one-shot execution mode to preserve the NodePowerResponse::Initiated reply contract and implement immediate and graceful power paths in src/bin/roxyd/handlers/power.rs. @sae-bom, thanks for the thoughtful alternative: a lower-cost in-process pending-operation (prepare a pending reboot/shutdown, write the Initiated response, then release the pending operation) combined with a small roxyd-local dispatch path to own response-write ordering.

To move this unblocked work forward I need an explicit preference so I can open an implementation PR that matches the maintainer/maintainers' expectation. Two concrete options are ready to implement:

1. Lightweight in-process pending operation (maintainer-preferred):

   • Handler prepares a pending reboot/shutdown that owns a release handle and waits.
   • roxyd dispatch prepares the pending operation, writes NodePowerResponse::Initiated, then releases it.
   • If response write fails, the release handle is dropped and no reboot/power-off happens.
   • Keeps everything in-process; requires a small roxyd-local dispatch path for node_power / flat reboot/shutdown compatibility.
   • Easier to review and avoids introducing a hidden internal CLI/subprocess for the first-pass migration.

2. Hidden one-shot roxyd execution mode (original plan):

   • Spawn a dedicated internal one-shot roxyd process that reports readiness, waits for explicit release, then calls reboot/poweroff.
   • Guarantees external execution boundary between the daemon and the immediate operation.
   • Slightly heavier (hidden CLI flag/subcommand and parent/child readiness/release protocol) but cleanly separates the action from the daemon lifetime.

Tradeoffs summarized:

• Option 1 is lower-cost, keeps code in-process, and aligns with your suggestion to minimize new cross-process machinery for a first-pass migration.
• Option 2 is more explicit about external execution boundary and may be preferable if you want strict separation now, but it introduces a hidden CLI surface and parent/child protocol.

Please confirm which option you prefer (reply with a short approval like “go ahead” or a 👍), and whether you want any constraints (naming, conventions, or specific reviewers). If you approve, I will:

• Open a PR implementing the chosen approach (including tests that avoid rebooting CI hosts by using a test seam/mock at the reboot boundary),
• Add focused handler and dispatch tests per the issue acceptance criteria,
• Request review from @sae-bom unless you prefer different reviewers.

If you’re happy with option 1 (in-process pending operation) I can start implementation immediately once you approve. If you prefer option 2, I will implement that instead. Also, if this is approved, please remove the "blocked" label so the issue can proceed. Thanks!

[zmrdltl]

Thanks, that makes sense to me.

I was trying to keep the existing upstream handler flow mostly unchanged, since review_protocol::request::handle() writes the response after the handler returns. That pushed the issue toward the separate one-shot roxyd execution path.

Your suggested in-process pending-operation approach looks simpler and more appropriate for this first-pass migration. I agree that the key requirement is response-write ordering, not a separate process boundary.

I will update have updated the issue body in that direction.