patch(activity backend): record device name/IP/MAC on per-miner bulk action detail

[rongxin-liu]

Problem

PR #25 shipped the per-miner batch detail RPC, but each row returns only the device UUID — a value the app surfaces nowhere else. Operators auditing a past bulk action have no way to recognize which miners were affected without cross-referencing UUIDs against another system. Frontend PR #60 renders whatever identifiers the backend hands it; today that's just the UUID.

What ships

Backend only. Follows the write-time capture convention already used by ActivityEntry.scope_label. Closes nothing; companion to #22 / #60.

• Record raw device-identity fields at first write. Five new nullable columns on command_on_device_log (custom_name, manufacturer, model, ip_address, mac_address), populated by UpsertCommandOnDeviceLog joining device + discovered_device. The read path composes the display name via fleetmanagement.ComposeDeviceName — the same helper the live fleet read path uses, so the audit log and live UI never disagree on what a miner is called.
• Immutable after first write. ON CONFLICT leaves the captured columns untouched so retries and the reaper-after-success path preserve the original audit values — even if the device was renamed or moved IPs between the first and second Upsert.
• Optional proto fields with UUID fallback. CommandBatchDeviceResult.{device_name, ip_address, mac_address} are optional string; historical rows (pre-migration) surface as unset so the frontend can fall back to UUID.

Where to review

Schema

• server/migrations/000037_add_device_snapshot_to_command_on_device_log.up.sql — five nullable TEXT columns. Nullable so historical rows stay NULL and the frontend can detect them.

SQL

• server/sqlc/queries/command.sql — UpsertCommandOnDeviceLog has a dev CTE that copies device.custom_name / device.mac_address / discovered_device.{manufacturer, model, ip_address} verbatim; no composition or rendering happens in SQL. ON CONFLICT DO UPDATE intentionally excludes the captured columns. ListBatchDeviceResults selects the five new columns.

Shared helper

• server/internal/domain/fleetmanagement/device_name.go — ComposeDeviceName(customName, manufacturer, model string) string is the single source of truth for the display-name rule. Both fleetmanagement/service.go (live fleet read) and command/service.go (audit log read) call it; the live-fleet call site was refactored to use it too so there's nowhere for the rule to drift.

Proto

• proto/minercommand/v1/command.proto — three optional string fields on CommandBatchDeviceResult (tags 5/6/7). optional is deliberate so the frontend can distinguish "never captured" (historical) from "captured empty." The field doc describes the consumer contract, not the composition recipe.

Service

• server/internal/domain/command/service.go — mapper reads the five raw sqlc fields, composes device_name via fleetmanagement.ComposeDeviceName, and passes ip_address / mac_address through using the existing ErrorMessage NullString-to-pointer pattern.

Tests

• server/internal/domain/fleetmanagement/device_name_test.go — table-driven unit test for ComposeDeviceName covering the five relevant cases (custom set, custom empty with both mfr/model, only mfr, only model, all empty).
• server/internal/domain/command/results_integration_test.go — TestGetCommandBatchDeviceResults_DeviceSnapshot covers the full invariant: first Upsert captures the fields; a rename + IP change; second Upsert (reaper-style) flips status/error_info but leaves the captured values alone. TestGetCommandBatchDeviceResults_HistoricalRowsOmitSnapshot covers backward compatibility by inserting a codl row directly with NULL captured columns and asserting the proto optionals surface as unset.

Test plan

• cd server && just test passes (the two new integration tests run against the TimescaleDB container and should both pass)
• Run a bulk action (e.g. "Blink LEDs") on a rack. Expand the batch in the activity log detail view (via PR feat(activity): per-miner bulk action detail modal [frontend] #60) and confirm name / IP / MAC appear alongside each device row
• Rename a miner (/miners/:id → rename) and expand the same batch again — original name still shown (audit invariant)
• Seed a pre-migration row manually (INSERT INTO command_on_device_log ... without captured columns) — verify the RPC returns unset optionals and the frontend falls back to UUID
• Kill a worker mid-command so the reaper fires; confirm the reaped row still has captured values (the reap path writes through the same UpsertCommandOnDeviceLog)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds an immutable “device identity snapshot” (name/IP/MAC) to per-miner bulk action results so operators can audit historical batches without cross-referencing UUIDs.

Changes:

• Add nullable snapshot columns to command_on_device_log and populate them on first write via UpsertCommandOnDeviceLog.
• Extend GetCommandBatchDeviceResults RPC to return optional device_name, ip_address, and mac_address.
• Add integration tests covering snapshot immutability and backward compatibility for pre-migration rows.

Reviewed changes

Copilot reviewed 6 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/sqlc/queries/command.sql	Populates snapshot fields on first upsert and returns them in batch result listing
server/migrations/000037_add_device_snapshot_to_command_on_device_log.up.sql	Adds nullable device snapshot columns to command_on_device_log
server/migrations/000037_add_device_snapshot_to_command_on_device_log.down.sql	Drops the snapshot columns on rollback
server/internal/domain/command/service.go	Maps new nullable DB fields to optional proto fields in the RPC response
server/internal/domain/command/results_integration_test.go	Adds integration tests for snapshot immutability + historical NULL behavior
proto/minercommand/v1/command.proto	Adds optional device_name, ip_address, mac_address fields to CommandBatchDeviceResult
server/generated/sqlc/models.go	sqlc regeneration to include new model fields
server/generated/sqlc/command.sql.go	sqlc regeneration for updated queries/row structs
server/generated/grpc/minercommand/v1/command.pb.go	Regenerated Go protobuf for new optional fields
client/src/protoFleet/api/generated/minercommand/v1/command_pb.ts	Regenerated TS protobuf for new optional fields

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0896d3c15

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

[github-actions]

🔐 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 (79e1347193b81875186824b1be6fd3a79579bfb3...6c4f7e3dff9ac5a6e082827649df9fac26638303, exact PR three-dot diff)
• Model: gpt-5.4

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

---

Review Summary

Overall Risk: LOW

Findings

No concrete security, correctness, or reliability defects stood out in the reviewed diff.

Notes

• Review scope was limited to .git/codex-review.diff for 79e1347193b81875186824b1be6fd3a79579bfb3...6c4f7e3dff9ac5a6e082827649df9fac26638303.
• The protobuf change is wire-compatible: it only adds optional fields with new tag numbers to CommandBatchDeviceResult.
• The new SQL stays parameterized and does not introduce SQL injection or command-injection surface.
• Auth behavior on GetCommandBatchDeviceResults appears unchanged and remains org-scoped through GetBatchHeaderForOrg.
• The main residual consideration is data sensitivity/retention: this patch intentionally persists historical device_name, ip_address, and mac_address in command_on_device_log. That looks deliberate, but it is worth confirming it matches your RBAC and retention expectations.
• I could not run Go tests in this sandbox because the filesystem is read-only and Go could not create its module/cache directories, so this is a static review only.

---

_{Generated by Codex Security Review |
Triggered by: @rongxin-liu |
Review workflow run}