proto: discovery + pairing wire contracts#257
Merged
Conversation
Adds the protocol surface for RFC 0001 phase 2 ahead of the server-side and agent-side implementation PRs. All RPCs return Unimplemented on the server until those PRs land; nothing on main calls them yet. fleetnodegateway/v1: - ReportDiscoveredDevices unary RPC + DiscoveredDeviceReport message (batch up to 1024). command_id field on the request correlates a batch to a server-issued ControlCommand so the gateway handler can forward it to the operator stream waiting on DiscoverOnFleetNode. - ControlStream bidi (already present; unchanged). fleetnodeadmin/v1: - PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices unary RPCs (session-only). - DiscoverOnFleetNode server-streaming RPC: operator triggers discovery on a specific fleet node; server proxies the request to the agent over ControlStream and forwards pairing.v1.DiscoverResponse batches back as they arrive. Reuses pairing.v1.DiscoverRequest so the operator UX is uniform with combined-mode pairing.PairingService.Discover. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
javascript
🔐 Codex Security Review
Review SummaryOverall Risk: HIGH Findings[HIGH] New admin RPCs are exposed but not implemented
[HIGH] Discovery report RPC is exposed but not implemented
NotesNo SQL, migration, infrastructure, plugin, Rust, or pool-address changes were present in the reviewed diff. The new interceptor entries look directionally correct: admin RPCs are session-only, the node report RPC is fleet-node authenticated, and discovery payload bodies are suppressed from debug logging. Generated by Codex Security Review | |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds protocol contracts for fleet-node discovery and pairing flows, with generated Go and TypeScript bindings updated for the gateway and admin services.
Changes:
- Adds
ReportDiscoveredDevicesto the fleet-node gateway API. - Adds fleet-node device pairing/listing and remote discovery admin RPCs.
- Regenerates Go Connect/protobuf and ProtoFleet TypeScript descriptors.
Reviewed changes
Copilot reviewed 2 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
proto/fleetnodegateway/v1/fleetnodegateway.proto |
Defines the new discovery report RPC and report messages. |
proto/fleetnodeadmin/v1/fleetnodeadmin.proto |
Defines new admin pairing, listing, and remote discovery RPCs. |
server/generated/grpc/fleetnodegateway/v1/fleetnodegateway.pb.go |
Generated Go protobuf bindings for gateway additions. |
server/generated/grpc/fleetnodegateway/v1/fleetnodegatewayv1connect/fleetnodegateway.connect.go |
Generated Go Connect client/server bindings for the gateway RPC. |
server/generated/grpc/fleetnodeadmin/v1/fleetnodeadmin.pb.go |
Generated Go protobuf bindings for admin additions. |
server/generated/grpc/fleetnodeadmin/v1/fleetnodeadminv1connect/fleetnodeadmin.connect.go |
Generated Go Connect client/server bindings for admin RPCs. |
client/src/protoFleet/api/generated/fleetnodegateway/v1/fleetnodegateway_pb.ts |
Generated TypeScript descriptors for gateway additions. |
client/src/protoFleet/api/generated/fleetnodeadmin/v1/fleetnodeadmin_pb.ts |
Generated TypeScript descriptors for admin additions. |
- Wrap DiscoverOnFleetNode's response in DiscoverOnFleetNodeResponse so buf-lint's RPC_RESPONSE_UNIQUE rule is satisfied (pairing.PairingService.Discover already uses DiscoverResponse). - Add gte=0 validation on ListFleetNodeDevicesRequest.fleet_node_id so negative filter values are rejected at the API boundary (Copilot review). - Add the new node-facing RPC (ReportDiscoveredDevices) to FleetNodeAuthenticatedProcedures and the four new admin RPCs (PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices, DiscoverOnFleetNode) to SessionOnlyProcedures. Without these the interceptor would route bearer-token requests through the wrong auth path. Codex HIGH + reviewer comment. - Reformat regenerated TS files with prettier (Client Lint CI was failing on two files). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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: 7073c53c5d
ℹ️ 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".
CI's generated-code check runs goimports -w . after buf generate and the import grouping differs from what my local just _gen-protos left. Re-running goimports locally so the committed bindings match what CI regenerates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- ReportDiscoveredDevicesRequest.command_id is now required
(min_len: 1, max_len: 128). The new server-initiated design only
accepts reports issued in response to a ControlCommand; empty
command_id can never bind to an operator stream and shouldn't be
on the wire.
- DiscoveredDeviceReport tightens defense-in-depth on the
fleet-node-to-server trust boundary:
- ip_address: string.ip = true (must parse as IPv4 or IPv6).
- port: CEL expression bounds the value to 1..65535 and rejects
non-numeric strings.
- url_scheme: enum'd to {http, https} via string.in.
Handler-side validateReport still enforces RFC1918/RFC4193 in
PR B; these proto-level rules just bounce the worst inputs
before the validator.Interceptor passes them down.
- Rename DiscoveredDeviceReport.type to driver_name. The schema and
domain code use driver_name as the plugin routing key
(discovered_device.type was dropped); the wire contract needs to
match so downstream pairing/control can resolve plugins for
remotely discovered devices. Codex P1.
- ReportDiscoveredDevices and DiscoverOnFleetNode added to
SensitiveBodyProcedures so request/response bodies stay out of
debug logs (LAN topology, device inventory, scan targets).
Codex LOW.
- Trim verbose proto comments per project style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mode support on DiscoverOnFleetNode: - IPListMode: agent runs TCP probe via plugin manager. - IPRangeMode: server expands to IPList before dispatch; agent sees an IPList. - NmapMode: agent shells out to the nmap binary bundled in the fleetnode release artifact. - MDNSMode: rejected with FailedPrecondition. Multicast UDP doesn't traverse the NAT/VLAN topology agents typically deploy into. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mcharles-square
approved these changes
May 19, 2026
ankitgoswami
added a commit
that referenced
this pull request
May 19, 2026
PR #257 landed the wire contracts on main, so this branch drops the proto + generated bindings + interceptor config entries that PR A now owns. What's left is the server-side implementation that consumes those contracts. What stays from the previous branch state: - Migration adding discovered_device.discovered_by_fleet_node_id + partial index. Renumbered to 000051 to avoid collision with 000050_add_curtailment_event_created_by which landed on main. - fleetnodepairing domain service + sqlstore + integration tests: PairDevice (CONFIRMED-only + intra-TX lock), UnpairDevice, ListPairs, ListDevicesForFleetNode, UpsertDiscoveredDevices with auto:* CTE reconciliation by (fleet_node, ip, port). - fleetnodegateway handler: ReportDiscoveredDevices (uses fleetnodeauth.Subject for org/node). - fleetnodeadmin handler: PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices. - pairing.PairDevices SSRF guard rejecting agent-reported rows. - RevokeFleetNode cleanup (deletes pairings, clears attribution). - GetActiveUnpairedDiscoveredDevices filter. - minerdiscovery model + discovered_device store DiscoveredByFleetNodeID plumbing. What adapted to the new wire: - DiscoveredDeviceReport.Type renamed to DriverName everywhere (model, store, handler, tests) to match the proto rename. - fleetd/main.go: wire fleetnodepairing.Service and pass it to both fleetnodegateway and fleetnodeadmin handler constructors. The new server-initiated components (fleetnodecontrol.Registry, ControlStream handler, DiscoverOnFleetNode admin handler) follow in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 19, 2026
Implement the server-initiated discovery flow that pairs with the proto contracts merged in #257. Operators trigger DiscoverOnFleetNode(fleet_node_id, pairing.v1.DiscoverRequest); the server dispatches a ControlCommand to the agent's open ControlStream, the agent reports devices via ReportDiscoveredDevices with the matching command_id, and the server forwards each batch back to the operator stream until the agent's ControlAck closes the call. - fleetnodecontrol.Registry: in-memory map[fleet_node_id]stream plus per-command_id event channels. Single-instance fleetd only. - ControlStream handler: Hello -> Accepted -> bidi pump of outgoing ControlCommand and incoming ControlAck. PublishAck wires acks to the in-flight command channel. - DiscoverOnFleetNode admin RPC: validates session + fleet node org scope, expands IPRange to IPList server-side, rejects MDNS/Nmap, encodes payload, streams batches until ack. - ReportDiscoveredDevices: when command_id is set, publishes each persisted batch to the registry so the operator stream wakes up. - Export ipscanner.GenerateIPsFromCIDR so other packages can reuse CIDR enumeration. Tests cover registry register/send/ack/disconnect, ControlStream hello/dispatch/ack/duplicate-stream rejection, DiscoverOnFleetNode happy path + no-stream + MDNS reject + IPRange expansion + viewer gate, and the ReportDiscoveredDevices command_id correlation path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
ankitgoswami
added a commit
that referenced
this pull request
May 27, 2026
PR #257 landed the wire contracts on main, so this branch drops the proto + generated bindings + interceptor config entries that PR A now owns. What's left is the server-side implementation that consumes those contracts. What stays from the previous branch state: - Migration adding discovered_device.discovered_by_fleet_node_id + partial index. Renumbered to 000051 to avoid collision with 000050_add_curtailment_event_created_by which landed on main. - fleetnodepairing domain service + sqlstore + integration tests: PairDevice (CONFIRMED-only + intra-TX lock), UnpairDevice, ListPairs, ListDevicesForFleetNode, UpsertDiscoveredDevices with auto:* CTE reconciliation by (fleet_node, ip, port). - fleetnodegateway handler: ReportDiscoveredDevices (uses fleetnodeauth.Subject for org/node). - fleetnodeadmin handler: PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices. - pairing.PairDevices SSRF guard rejecting agent-reported rows. - RevokeFleetNode cleanup (deletes pairings, clears attribution). - GetActiveUnpairedDiscoveredDevices filter. - minerdiscovery model + discovered_device store DiscoveredByFleetNodeID plumbing. What adapted to the new wire: - DiscoveredDeviceReport.Type renamed to DriverName everywhere (model, store, handler, tests) to match the proto rename. - fleetd/main.go: wire fleetnodepairing.Service and pass it to both fleetnodegateway and fleetnodeadmin handler constructors. The new server-initiated components (fleetnodecontrol.Registry, ControlStream handler, DiscoverOnFleetNode admin handler) follow in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 27, 2026
Implement the server-initiated discovery flow that pairs with the proto contracts merged in #257. Operators trigger DiscoverOnFleetNode(fleet_node_id, pairing.v1.DiscoverRequest); the server dispatches a ControlCommand to the agent's open ControlStream, the agent reports devices via ReportDiscoveredDevices with the matching command_id, and the server forwards each batch back to the operator stream until the agent's ControlAck closes the call. - fleetnodecontrol.Registry: in-memory map[fleet_node_id]stream plus per-command_id event channels. Single-instance fleetd only. - ControlStream handler: Hello -> Accepted -> bidi pump of outgoing ControlCommand and incoming ControlAck. PublishAck wires acks to the in-flight command channel. - DiscoverOnFleetNode admin RPC: validates session + fleet node org scope, expands IPRange to IPList server-side, rejects MDNS/Nmap, encodes payload, streams batches until ack. - ReportDiscoveredDevices: when command_id is set, publishes each persisted batch to the registry so the operator stream wakes up. - Export ipscanner.GenerateIPsFromCIDR so other packages can reuse CIDR enumeration. Tests cover registry register/send/ack/disconnect, ControlStream hello/dispatch/ack/duplicate-stream rejection, DiscoverOnFleetNode happy path + no-stream + MDNS reject + IPRange expansion + viewer gate, and the ReportDiscoveredDevices command_id correlation path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 28, 2026
PR #257 landed the wire contracts on main, so this branch drops the proto + generated bindings + interceptor config entries that PR A now owns. What's left is the server-side implementation that consumes those contracts. What stays from the previous branch state: - Migration adding discovered_device.discovered_by_fleet_node_id + partial index. Renumbered to 000051 to avoid collision with 000050_add_curtailment_event_created_by which landed on main. - fleetnodepairing domain service + sqlstore + integration tests: PairDevice (CONFIRMED-only + intra-TX lock), UnpairDevice, ListPairs, ListDevicesForFleetNode, UpsertDiscoveredDevices with auto:* CTE reconciliation by (fleet_node, ip, port). - fleetnodegateway handler: ReportDiscoveredDevices (uses fleetnodeauth.Subject for org/node). - fleetnodeadmin handler: PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices. - pairing.PairDevices SSRF guard rejecting agent-reported rows. - RevokeFleetNode cleanup (deletes pairings, clears attribution). - GetActiveUnpairedDiscoveredDevices filter. - minerdiscovery model + discovered_device store DiscoveredByFleetNodeID plumbing. What adapted to the new wire: - DiscoveredDeviceReport.Type renamed to DriverName everywhere (model, store, handler, tests) to match the proto rename. - fleetd/main.go: wire fleetnodepairing.Service and pass it to both fleetnodegateway and fleetnodeadmin handler constructors. The new server-initiated components (fleetnodecontrol.Registry, ControlStream handler, DiscoverOnFleetNode admin handler) follow in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 28, 2026
Implement the server-initiated discovery flow that pairs with the proto contracts merged in #257. Operators trigger DiscoverOnFleetNode(fleet_node_id, pairing.v1.DiscoverRequest); the server dispatches a ControlCommand to the agent's open ControlStream, the agent reports devices via ReportDiscoveredDevices with the matching command_id, and the server forwards each batch back to the operator stream until the agent's ControlAck closes the call. - fleetnodecontrol.Registry: in-memory map[fleet_node_id]stream plus per-command_id event channels. Single-instance fleetd only. - ControlStream handler: Hello -> Accepted -> bidi pump of outgoing ControlCommand and incoming ControlAck. PublishAck wires acks to the in-flight command channel. - DiscoverOnFleetNode admin RPC: validates session + fleet node org scope, expands IPRange to IPList server-side, rejects MDNS/Nmap, encodes payload, streams batches until ack. - ReportDiscoveredDevices: when command_id is set, publishes each persisted batch to the registry so the operator stream wakes up. - Export ipscanner.GenerateIPsFromCIDR so other packages can reuse CIDR enumeration. Tests cover registry register/send/ack/disconnect, ControlStream hello/dispatch/ack/duplicate-stream rejection, DiscoverOnFleetNode happy path + no-stream + MDNS reject + IPRange expansion + viewer gate, and the ReportDiscoveredDevices command_id correlation path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 28, 2026
PR #257 landed the wire contracts on main, so this branch drops the proto + generated bindings + interceptor config entries that PR A now owns. What's left is the server-side implementation that consumes those contracts. What stays from the previous branch state: - Migration adding discovered_device.discovered_by_fleet_node_id + partial index. Renumbered to 000051 to avoid collision with 000050_add_curtailment_event_created_by which landed on main. - fleetnodepairing domain service + sqlstore + integration tests: PairDevice (CONFIRMED-only + intra-TX lock), UnpairDevice, ListPairs, ListDevicesForFleetNode, UpsertDiscoveredDevices with auto:* CTE reconciliation by (fleet_node, ip, port). - fleetnodegateway handler: ReportDiscoveredDevices (uses fleetnodeauth.Subject for org/node). - fleetnodeadmin handler: PairDeviceToFleetNode, UnpairDevice, ListFleetNodeDevices. - pairing.PairDevices SSRF guard rejecting agent-reported rows. - RevokeFleetNode cleanup (deletes pairings, clears attribution). - GetActiveUnpairedDiscoveredDevices filter. - minerdiscovery model + discovered_device store DiscoveredByFleetNodeID plumbing. What adapted to the new wire: - DiscoveredDeviceReport.Type renamed to DriverName everywhere (model, store, handler, tests) to match the proto rename. - fleetd/main.go: wire fleetnodepairing.Service and pass it to both fleetnodegateway and fleetnodeadmin handler constructors. The new server-initiated components (fleetnodecontrol.Registry, ControlStream handler, DiscoverOnFleetNode admin handler) follow in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ankitgoswami
added a commit
that referenced
this pull request
May 28, 2026
Implement the server-initiated discovery flow that pairs with the proto contracts merged in #257. Operators trigger DiscoverOnFleetNode(fleet_node_id, pairing.v1.DiscoverRequest); the server dispatches a ControlCommand to the agent's open ControlStream, the agent reports devices via ReportDiscoveredDevices with the matching command_id, and the server forwards each batch back to the operator stream until the agent's ControlAck closes the call. - fleetnodecontrol.Registry: in-memory map[fleet_node_id]stream plus per-command_id event channels. Single-instance fleetd only. - ControlStream handler: Hello -> Accepted -> bidi pump of outgoing ControlCommand and incoming ControlAck. PublishAck wires acks to the in-flight command channel. - DiscoverOnFleetNode admin RPC: validates session + fleet node org scope, expands IPRange to IPList server-side, rejects MDNS/Nmap, encodes payload, streams batches until ack. - ReportDiscoveredDevices: when command_id is set, publishes each persisted batch to the registry so the operator stream wakes up. - Export ipscanner.GenerateIPsFromCIDR so other packages can reuse CIDR enumeration. Tests cover registry register/send/ack/disconnect, ControlStream hello/dispatch/ack/duplicate-stream rejection, DiscoverOnFleetNode happy path + no-stream + MDNS reject + IPRange expansion + viewer gate, and the ReportDiscoveredDevices command_id correlation path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Unimplementedon the server until feat(fleetnode): server-initiated discovery via ControlStream #235 (server) and feat(fleetnode): discovery scan + ReportDiscoveredDevices agent #256 (agent) land.fleetnodegateway/v1
ReportDiscoveredDevicesunary batch RPC,DiscoveredDeviceReportmessage (max_items=1024).command_idfield onReportDiscoveredDevicesRequest(optional, max_len=128) so an agent reporting batches in response to a server-issuedControlCommandcan echo the id, letting the server route batches to the operator stream waiting onDiscoverOnFleetNode.ControlStreambidi already in the file (unchanged); listed here so reviewers know both halves of the wire are present.fleetnodeadmin/v1
PairDeviceToFleetNode,UnpairDevice,ListFleetNodeDevicesunary (session-only auth).DiscoverOnFleetNode(DiscoverOnFleetNodeRequest) returns (stream pairing.v1.DiscoverResponse).pairing.v1.DiscoverRequestso the operator UX is uniform with combined-modepairing.PairingService.Discover.ControlStream(handler shipping in feat(fleetnode): server-initiated discovery via ControlStream #235) and forwardspairing.v1.DiscoverResponsebatches as they arrive.Why this is safe to merge alone
maingetCodeUnimplemented.maincalls these RPCs.ReportDiscoveredDevices.command_idis optional (proto3 default-empty); existing callers (none yet) unaffected.Merge order
Test plan
PATH=./bin:$PATH just _gen-protos— clean regen, only affected files diff (gateway + admin proto + Go + TS).PATH=./bin:$PATH go build ./server/...— compiles (handlers stay onUnimplementedFleetNodeGatewayServiceHandler/UnimplementedFleetNodeAdminServiceHandlerfor the new RPCs).PATH=./bin:$PATH go vet ./server/...— clean.cd server && PATH=../bin:$PATH golangci-lint run -c .golangci.yaml ./generated/grpc/fleetnodegateway/... ./generated/grpc/fleetnodeadmin/...— 0 issues.🤖 Generated with Claude Code