Skip to content

feat(fleetnode): pair discovered miners (orchestration + operator RPCs)#396

Merged
ankitgoswami merged 1 commit into
mainfrom
fleetnode-pairing-orchestration
Jun 11, 2026
Merged

feat(fleetnode): pair discovered miners (orchestration + operator RPCs)#396
ankitgoswami merged 1 commit into
mainfrom
fleetnode-pairing-orchestration

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Completes pairing miners discovered via fleet nodes, stacked on the foundation in #388 . With this, an operator can take a fleet-node-discovered miner from a discovered_device dead-end through to a fully paired, node-owned device.

What is in here

  • Persist pairing results. The control registry routes per-device pair results to the waiting operator session (PublishPairResults). ResolvePairTargets builds the batch from the node's not-yet-paired devices, and PersistFleetNodePairResult records each result in one transaction:
    • PAIRED: create the device, bind it to the node (fleet_node_device) before marking device_pairing PAIRED so the cloud-paired guard never sees a PAIRED-but-unbound row, store encrypted credentials for basic-auth drivers, seed an ACTIVE status.
    • AUTH_NEEDED / AUTH_FAILED: persist an AUTHENTICATION_NEEDED device for credential retry.
    • ERROR: persist nothing.
      pairDeviceLocked is factored out of PairDevice so both share the binding logic inside a caller-owned transaction.
  • Operator RPCs. The gateway ReportPairedDevices handler binds the node's results to the in-flight command and routes them up. The server-streaming PairDiscoveredDevicesOnFleetNode dispatches a pair AgentCommand down the ControlStream and streams per-device outcomes as it persists them (mirroring DiscoverOnFleetNode); ListFleetNodeDiscoveredDevices enumerates candidates. fleetnode:manage./Gated by fleetnode:read (listing) and fleetnode:manage (pairing); PairDiscoveredDevicesOnFleetNode additionally requires miner:pair, consistent with every other miner-onboarding path (e.g. ForemanImport), since it authenticates and onboards miners./ fleetnode:manage.

Testing

  • go build ./... clean; client tsc clean.
  • Node tests pass with -race.
  • Fleetnode domain tests against the live test DB cover PersistFleetNodePairResult (asymmetric PAIRED, basic-auth PAIRED with creds stored, AUTH_NEEDED then retry to PAIRED, foreign-node rejection, ERROR persists nothing) and ResolvePairTargets.
  • Admin handler tests drive the full server-streaming pair path against a simulated node and assert the persisted device_pairing = PAIRED + fleet_node_device binding, plus no-stream / no-targets / permission cases; gateway tests cover ReportPairedDevices routing.

The fake-rig fixtures need no change: the node calls the identical plugin PairDevice contract the cloud uses (only the SecretBundle source differs). A full Docker E2E run needs a live just dev stack with an enrolled, connected fleet node; the demo script is that manual-verification path.

🤖 Generated with Claude Code

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

github-actions Bot commented Jun 4, 2026 •
edited
Loading

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 (4614144c9f5d37d006af972fe660d68fae1c3bf3...e219e36f34a5d0bc28b12fb3beabceca0a1d098b, exact PR three-dot diff)
  • Model: gpt-5.5

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

Review Summary

Overall Risk: NONE

Findings

No security, correctness, or reliability findings in the scoped PR diff.

Notes

Reviewed .git/codex-review.diff only. I specifically checked the new fleet-node pair/report flow, credential handling, report scoping, auth interceptor config, sqlc queries, migration, generated protobuf outputs, and cryptostealing/pool-hijack indicators.

I did not run the test suite because the environment is read-only; this review is based on static inspection of the supplied diff and surrounding code.

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

@github-actions github-actions Bot added the shared label Jun 4, 2026
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 924106b to c4e448f Compare June 4, 2026 22:13
ankitgoswami added a commit that referenced this pull request Jun 4, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
4b441a7 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
a8e23d9 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 4b441a7 to a8e23d9 Compare June 5, 2026 16:11
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
9632479 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from a8e23d9 to 9632479 Compare June 5, 2026 16:21
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
442d299 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 9632479 to 442d299 Compare June 5, 2026 16:24
@ankitgoswami ankitgoswami mentioned this pull request Jun 5, 2026
Merged
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
d88cbbb 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 91f9f5b to 090667a Compare June 5, 2026 16:40
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
9218812 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 090667a to f7caf78 Compare June 5, 2026 16:43
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
0297402 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from f7caf78 to 1731a5b Compare June 5, 2026 16:47
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
72f3aa2 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 1731a5b to f59394f Compare June 5, 2026 17:08
ankitgoswami added a commit that referenced this pull request Jun 5, 2026
@ankitgoswami @claude
fix(fleetnode): address pairing review (cred bounds, dedup, cloud-pai…
00b19eb 
…ring guard)

From the multi-agent review of #396:
- Skip plugin default credentials that exceed the FleetNodePairResult caps before
  pairing, so an oversized default can't fail ReportPairedDevices validation for
  the whole batch (used_username/used_password were the gap left by the earlier
  identity-field clamp).
- requestedPairResults consumes each target on first match, so a node replaying an
  in-target identifier (up to its quota) can't amplify into repeated heavy persists.
- Guard the AUTHENTICATION_NEEDED downgrade with DeviceHasActiveCloudPairing so a
  node auth-needed report can't downgrade a device that became cloud-paired since
  target resolution; regression test added.
- AckFailure maps ACK_CODE_REPORT_FAILED to a clearer 'some may have been applied,
  re-list' message instead of an opaque internal error.
- Correct the ListFleetNodeDiscoveredDevices limit proto comment (0 = default 1024,
  not 'no limit') to match the handler clamp.
- Add gateway ReportPairedDevices handler tests; add the missing AAA // Arrange.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from f59394f to 3b8fa24 Compare June 5, 2026 17:21
chatgpt-codex-connector[bot]

This comment was marked as outdated.

Sign in to view

chatgpt-codex-connector[bot]

This comment was marked as outdated.

Sign in to view

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes

@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: 45d63e8c8d

ℹ️ 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/pair_discovered.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from c2d3d6d to c84a540 Compare June 5, 2026 22:18
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes

@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: c84a540026

ℹ️ 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/pair_discovered.go Outdated
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch 2 times, most recently from 57954a1 to 1c24fe5 Compare June 5, 2026 22:29
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes

@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: 1c24fe5020

ℹ️ 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/pair_discovered.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 1c24fe5 to 9bb671e Compare June 5, 2026 23:01
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes

@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

proto-fleet/server/cmd/fleetnode/pair.go

Lines 258 to 261 in 9bb671e

_, err := client.ReportPairedDevices(callCtx, connect.NewRequest(&pb.ReportPairedDevicesRequest{
CommandId: commandID,
Results: chunk,
}))

P2 Badge Check rejected pair reports before acknowledging

When the gateway persists only part of this chunk (for example PersistFleetNodePairResult fails for one paired miner), ReportPairedDevices returns a successful RPC with RejectedCount > 0, but this caller discards the response and later sends an OK ack for the command. In that scenario the node considers the result uploaded even though the cloud did not store/bind the miner it just paired, so the failed result is not retried; treat any rejected pair result as a report failure before acknowledging the command. Fresh evidence in this revision is that the current node code still ignores the response while the gateway now reports persistence drops via RejectedCount.

ℹ️ 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".

@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch 5 times, most recently from 6d87253 to 688b5e0 Compare June 9, 2026 17:28
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 9, 2026
View reviewed changes

@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: 688b5e05b3

ℹ️ 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/pair_discovered.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 688b5e0 to 8327449 Compare June 9, 2026 18:28
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 9, 2026
View reviewed changes

@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: 8327449d14

ℹ️ 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/pair_discovered.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 8327449 to 230a870 Compare June 10, 2026 14:49
@ankitgoswami ankitgoswami enabled auto-merge (squash) June 11, 2026 15:55
rongxin-liu
rongxin-liu approved these changes Jun 11, 2026
View reviewed changes
Comment thread server/cmd/fleetnode/pair.go
Comment thread server/internal/handlers/fleetnode/gateway/handler.go
Comment thread server/internal/domain/fleetnode/pairing/pair_discovered.go
Comment thread server/internal/domain/stores/sqlstores/device.go
Comment thread server/internal/domain/fleetnode/control/stream.go Outdated
Comment thread server/internal/domain/fleetnode/pairing/models.go Outdated
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 230a870 to 21ce9a8 Compare June 11, 2026 19:21
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 11, 2026
View reviewed changes

@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: 21ce9a8fe4

ℹ️ 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/pair_discovered.go
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch 2 times, most recently from e98f25f to 6e99b8c Compare June 11, 2026 19:29
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 11, 2026
View reviewed changes

@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: 6e99b8c8f7

ℹ️ 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/fleetnodepairing.sql Outdated
@ankitgoswami @claude
feat(fleetnode): pair discovered miners (orchestration + operator RPCs)
e219e36 
Cloud orchestration for pairing fleet-node-discovered miners over the
ControlStream, built on the #388 foundation (AgentCommand pair arm, node pairer,
SQL listing + cloud-dial guards).

Server domain:
- PersistFleetNodePairResult records each device's outcome in its own
  transaction: PAIRED creates the device, binds it to the node before marking
  device_pairing PAIRED (so the cloud-paired guard never sees a PAIRED-but-unbound
  row), seeds an ACTIVE status, and stores encrypted credentials for password-auth
  drivers only; AUTH_NEEDED/AUTH_FAILED persist AUTHENTICATION_NEEDED for retry;
  ERROR persists nothing. It never downgrades an already-PAIRED device (race with
  the cloud or another node) and preserves a learned serial when a report omits it.
- ResolvePairTargets builds the batch from the node's not-yet-paired devices;
  pair-all without credentials excludes AUTHENTICATION_NEEDED rows so a capped
  batch can't starve never-attempted devices on re-issue for large fleets.
- pairDeviceLocked is factored out of PairDevice so both share the binding logic.

Registry + dispatch (authoritative, decoupled from the operator stream):
- The gateway ReportPairedDevices handler is the source of truth: it admits +
  scopes results to the dispatched targets (consuming each to bar replay), rejects
  empty batches, enforces a per-command quota = target count, persists each result
  before acking, and forwards only successfully-persisted results (with the
  persisted status) for live display.
- PublishPairResults / report-kind admission keep discovery and pairing reports
  from cross-admitting.
- The operator command is dispatched on a disconnect-immune context so pairing
  runs to completion server-side and the gateway keeps persisting even if the
  operator/browser disconnects; the admin stream is a best-effort observer.

Operator admin RPCs:
- ListFleetNodeDiscoveredDevices (paged) and streaming PairDiscoveredDevicesOnFleetNode,
  gated by miner:pair + fleetnode:manage, session-only, with credential bodies
  redacted/suppressed in logs.
- DevicePairingResult.pairing_status uses the shared fleetmanagement PairingStatus
  enum.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ankitgoswami ankitgoswami force-pushed the fleetnode-pairing-orchestration branch from 6e99b8c to e219e36 Compare June 11, 2026 19:41
@ankitgoswami ankitgoswami merged commit 4e477bd into main Jun 11, 2026
76 checks passed
@ankitgoswami ankitgoswami deleted the fleetnode-pairing-orchestration branch June 11, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@rongxin-liu rongxin-liu rongxin-liu approved these changes

Assignees

@ankitgoswami ankitgoswami

Labels

client javascript Pull requests that update javascript code server shared

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ankitgoswami @rongxin-liu