Skip to content

feat(fleetnode): route miner commands to fleet-node-paired devices#390

Open
ankitgoswami wants to merge 1 commit into
mainfrom
fleetnode-commands-p2
Open

feat(fleetnode): route miner commands to fleet-node-paired devices#390
ankitgoswami wants to merge 1 commit into
mainfrom
fleetnode-commands-p2

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Operator commands now reach miners paired via fleet nodes. A command issued through the existing RPCs/UI against a fleet-node-paired device routes down that node's ControlStream; the node reconstructs the LAN miner and executes the action, and the result returns as a ControlAck. The entire command service / queue / status / permissions / curtailment pipeline is unchanged — this is RFC 0001's remote-node Miner adapter slotted in at miner resolution.

Foundations #389 (ControlStream command plane) and #388 (pairing) are merged; this now sits directly on main. It shares the AgentCommand envelope with the pairing effort: pair = 2 (from #388), miner_command = 3 (this PR).

What changed

  • proto: pairing.v1.MinerCommand — a oneof over reboot / start / stop / blink-LED / curtail / uncurtail / set-cooling-mode / set-power-target, plus a MinerConnectionDescriptor (cloud supplies connection coords; the node has no device DB) and an opaque credential field. Wired as AgentCommand.miner_command = 3. Added ACK_CODE_UNAUTHENTICATED / ACK_CODE_UNIMPLEMENTED.
  • server adapter (internal/domain/miner/remotenode): an interfaces.Miner whose methods marshal a MinerCommand, call registry.SendCommand, and map the ControlAck to the method's error contract.
  • resolution (internal/domain/miner): new GetActiveFleetNodeForDevice query; GetMinerFromDeviceIdentifier checks fleet-node ownership first and returns the adapter, else falls through to the direct PluginMiner. Wired in fleetd via WithCommandSender.
  • node (cmd/fleetnode): handleMinerCommand builds a control device from the descriptor via the plugin driver and runs the action, behind a credential-provider seam.

Reliability & hardening

  • Offline node is retryable: ErrNoActiveStream → Unavailable (not permanent FailedPrecondition), so a transient node reconnect doesn't drop a queued command. BUSY → ResourceExhausted, paced by a per-node gate.
  • Immediate revocation: the remote miner is re-resolved per command (not cached), so unpair / fleet-node revoke takes effect on the next command rather than after the miner-cache TTL.
  • No duplicate execution: the node command timeout (25s) stays below the server worker budget (30s), so a slow command can't be retried while the node is still running it.
  • Input validation at the node: the unmarshaled MinerCommand is protovalidate-checked at the control boundary and the dial target is constrained (valid private/loopback IP, http/https scheme) before any plugin dial; undefined enum values (cooling / power / curtail) are rejected with BAD_REQUEST; the untrusted node ack message is clamped server-side before it is persisted.
  • Stale routing: GetActiveFleetNodeForDevice excludes soft-deleted discovery rows.
  • Gateway input validation: inbound ControlAck is protovalidate-checked at the gateway boundary (malformed or oversized acks are dropped, not routed); the two direct-dial coordinate queries also exclude soft-deleted discovered_device rows, matching the fleet-node query.
  • Tests cover the per-miner pool-ceiling BUSY path and concurrent-command routing.

Scope / boundary

  • Works end to end now: the no-secret virtual driver (the just dev fake miners).
  • Authenticated drivers (proto, antminer): route correctly but do not execute yet — they need credential provisioning (the node's per-org key / token minting), which is the pairing / Phase-3 work. Tracked in Fleet-node authenticated-driver credential delivery (post #390) #402, including making the missing-credential failure permanent rather than retryable.
  • Deferred (Phase 3): firmware update, log download, password change, unpair; read / telemetry methods. The adapter returns Unimplemented for these.

Test plan

  • go test -race ./cmd/fleetnode/... ./internal/domain/miner/... — green: adapter action encoding for all 8 commands + ack→error mapping, node executor via gomock, and DB-backed resolution against the dev Postgres. Full server build + golangci-lint clean.

Manual end to end (virtual driver)

just dev; enroll/confirm a fleet node; discover and pair a virtual device; issue Reboot/Curtail/StartMining and confirm it routes over the ControlStream (node logs control command received, kind=miner_command), executes against the fake miner, and the batch reports SUCCESS. Verify an offline node retries then fails cleanly, and that a command issued during a discovery on the same node still succeeds.

Follow-ups

🤖 Generated with Claude Code

@github-actions github-actions Bot added javascript Pull requests that update javascript code client server shared labels Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (75933fce45c269e41e0ba2b30cd33825b0389fa7...97d7288164591372dc1e30485d7b3af94ea076e4, exact PR three-dot diff)
  • Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: HIGH

Findings

[HIGH] Fleet-node miner commands are dispatched without authentication material

  • Category: Auth
  • Location: server/internal/domain/miner/service.go:203
  • Description: tryFleetNodeMiner now routes every paired fleet-node device through remotenode.New, but the descriptor is built without any credential or signer material. The new SQL query also does not select miner_credentials, and the fleet node’s default nodeSecretProvider returns an empty sdk.SecretBundle. Real drivers do not accept that: Proto requires a BearerToken, and Antminer requires UsernamePassword.
  • Impact: Successfully paired fleet-node miners cannot execute reboot/start/stop/curtail/power/cooling commands. Depending on the driver, this is reported as auth/internal failure and burns queue retries instead of executing, making the new remote command path effectively work only for no-auth test/virtual drivers.
  • Recommendation: Complete the auth handoff before enabling this route for real devices. Include stored encrypted credentials in GetActiveFleetNodeForDevice and implement node-side decryption/secret reconstruction, and generate Proto bearer tokens from the fleet node’s miner-signing private key. Add end-to-end tests for at least one basic-auth driver and one Proto/asymmetric driver.

Notes

No SQL injection, command injection, pool hijacking, or protobuf wire-format breakage was evident in the reviewed diff. The main issue is that the new command routing path crosses the fleet-node/plugin auth boundary without carrying the credentials required by the existing drivers.


Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run

ankitgoswami added a commit that referenced this pull request Jun 4, 2026
Bring #390 onto the reconciled #389 (which now includes main + #365). Only the
shared pairing proto conflicted: keep the MinerCommand additions and the
AgentCommand envelope (field 2 = miner_command now implemented, 3 = pair still
reserved), then regenerate pairing Go/TS with the pinned toolchain. All other
files merged cleanly; #365's discovery refactor and the AgentCommand migration
arrive via #389.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ankitgoswami added a commit that referenced this pull request Jun 4, 2026
reportBearingCommand routes discovery to the exclusive single-flight slot and
per-miner (and malformed) commands to the broader pool. This lives with #390
because it needs the MinerCommand envelope arm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ankitgoswami added a commit that referenced this pull request Jun 4, 2026
reportBearingCommand routes discovery to the exclusive single-flight slot and
per-miner (and malformed) commands to the broader pool. This lives with #390
because it needs the MinerCommand envelope arm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from ca1a0a1 to 72d141c Compare June 4, 2026 20:52
ankitgoswami added a commit that referenced this pull request Jun 4, 2026
reportBearingCommand routes discovery to the exclusive single-flight slot and
per-miner (and malformed) commands to the broader pool. This lives with #390
because it needs the MinerCommand envelope arm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 72d141c to 24732be Compare June 4, 2026 23:00
Base automatically changed from fleetnode-commands to main June 5, 2026 14:48
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
reportBearingCommand routes discovery to the exclusive single-flight slot and
per-miner (and malformed) commands to the broader pool. This lives with #390
because it needs the MinerCommand envelope arm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 24732be to 900f48b Compare June 5, 2026 14:51
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
Resolves the #388 (pairing) <-> #390 (miner commands) overlap on the shared
AgentCommand envelope and control loop:
- AgentCommand: #388 shipped `pair = 2`, so `miner_command` moves to field 3
  (the slot main reserved for it). Pre-release, nothing depends on the old number.
- handleCommand dispatches both the pair (#388) and miner_command (#390) arms.
- run.go wires the pairer, driverGetter, and minerSecrets off the shared plugin manager.
- Regenerated pairing/fleetnodegateway Go + TS; sqlc unchanged-but-verified.
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 1ae3c3c to 1c95e7d Compare June 5, 2026 20:11
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch 2 times, most recently from 1e91f50 to 7437cc0 Compare June 5, 2026 21:01
@ankitgoswami ankitgoswami marked this pull request as ready for review June 5, 2026 21:30
@ankitgoswami ankitgoswami requested a review from a team as a code owner June 5, 2026 21:30
Copilot AI review requested due to automatic review settings June 5, 2026 21:30

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7437cc03a8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/cmd/fleetnode/minercommand.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables operator miner commands (reboot/curtail/start/stop/etc.) to reach devices paired via fleet nodes by routing command execution over the fleet node ControlStream, while keeping the existing command queue/permissions/status pipeline unchanged.

Changes:

  • Adds pairing.v1.MinerCommand + MinerConnectionDescriptor in the AgentCommand envelope, plus new AckCode values (UNAUTHENTICATED, UNIMPLEMENTED) for better server/node error semantics.
  • Adds a server-side remotenode interfaces.Miner adapter and resolution logic (GetActiveFleetNodeForDevice) so fleet-node-owned devices route over the control plane instead of direct-dial.
  • Adds node-side miner command execution (cmd/fleetnode/minercommand.go) with protovalidate validation and dial-target constraints, plus gateway-side inbound ControlAck validation/dropping.

Reviewed changes

Copilot reviewed 17 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
server/sqlc/queries/miner_service.sql Adds GetActiveFleetNodeForDevice resolution query; tightens direct-dial queries to ignore soft-deleted discovered_device rows.
server/internal/handlers/fleetnode/gateway/handler.go Validates inbound ControlAck at the gateway trust boundary and drops invalid/oversized acks.
server/internal/handlers/fleetnode/gateway/handler_controlstream_test.go Tests that invalid acks are dropped and do not poison command completion.
server/internal/domain/miner/service.go Adds command routing configuration and fleet-node-aware miner resolution via tryFleetNodeMiner.
server/internal/domain/miner/resolution_test.go Integration test asserting node-paired devices resolve to the remote-node miner adapter and dispatch via sender.
server/internal/domain/miner/remotenode/miner.go Implements remote-node Miner adapter, ack→error mapping, and ack error message clamping.
server/internal/domain/miner/remotenode/miner_test.go Unit tests for encoding, ack mapping, retryability, and clamp behavior.
server/internal/domain/miner/remotenode/gate.go / gate_test.go Per-fleet-node concurrency limiter to pace large batches and avoid node BUSY rejections.
server/cmd/fleetnode/run.go Adds seams for driver lookup and secret provisioning used by miner command execution.
server/cmd/fleetnode/minercommand.go / minercommand_test.go Node-side decoding/validation, device reconstruction, action execution, and ack classification.
server/cmd/fleetnode/control.go / control_test.go Dispatches AgentCommand.miner_command and tests BUSY behavior at worker pool ceiling.
server/cmd/fleetd/main.go Wires MinerService.WithCommandSender to enable routing in fleetd.
proto/pairing/v1/pairing.proto Adds MinerCommand + descriptor into AgentCommand envelope.
proto/fleetnodegateway/v1/fleetnodegateway.proto Adds new AckCode enum values used for command semantics.
Generated Go/TS (server/generated/..., client/src/.../generated/...) Regenerated outputs reflecting the proto/sqlc changes.

Comment thread server/internal/domain/miner/service.go
Comment thread server/internal/domain/miner/remotenode/miner.go Outdated
Comment thread server/cmd/fleetnode/minercommand.go Outdated
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 7437cc0 to 77388bd Compare June 5, 2026 21:41

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77388bdbde

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/miner/service.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 77388bd to 58b3fec Compare June 5, 2026 21:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58b3fec8d1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/miner/remotenode/miner.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 58b3fec to 6b1d691 Compare June 5, 2026 22:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b1d691087

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/sqlc/queries/miner_service.sql Outdated
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch 2 times, most recently from 91652b6 to 03cc3c2 Compare June 5, 2026 22:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03cc3c227a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/miner/service.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from 03cc3c2 to 07f2977 Compare June 5, 2026 23:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07f2977eb3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/miner/service.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch 2 times, most recently from 35b96e8 to 6e1cdb9 Compare June 11, 2026 20:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e1cdb93cf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/fleetnode/pairing/service.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch 2 times, most recently from 5c1aa4e to bc427d5 Compare June 12, 2026 19:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc427d5ff2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/sqlc/queries/miner_service.sql Outdated
Operators issue commands (reboot, curtail, start/stop, cooling, power) against
fleet-node-paired miners through the existing RPC/queue pipeline; this routes them
over the node's ControlStream instead of dialing the miner directly.

- AgentCommand.miner_command (field 3) envelope + MinerCommand/descriptor/action protos.
- remotenode.Miner adapter implements interfaces.Miner: marshals the command, sends it
  via registry.SendCommand, maps the ack to an error. Offline node -> Unavailable
  (retryable); BUSY -> ResourceExhausted; untrusted ack text is clamped server-side.
- Resolution: GetActiveFleetNodeForDevice routes fleet-node-owned devices (excluding
  soft-deleted discovery rows); the remote miner is re-resolved per command (not cached)
  so unpair/revoke takes effect immediately.
- Node executor: handleMinerCommand builds the device via the plugin driver and runs the
  action; enum values are validated (undefined -> BAD_REQUEST); a per-node command gate
  paces large batches; node command timeout (25s) stays below the server worker budget so
  a slow command can't be retried while still executing.
- PerNodeLimiter, ACK_CODE_UNAUTHENTICATED/UNIMPLEMENTED, and tests incl. the per-miner
  pool-ceiling BUSY path and concurrent-command routing.

Authenticated drivers (proto/antminer) still need credential provisioning (pairing);
the no-secret virtual driver works end to end now.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-commands-p2 branch from bc427d5 to 97d7288 Compare June 12, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client javascript Pull requests that update javascript code server shared

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants