Skip to content

feat(rollout): release channel domain service (channels and scopes) - #1016

Draft
rl-block wants to merge 1 commit into
rollout/01b-release-channel-schemafrom
rollout/01c-release-channel-domain
Draft

feat(rollout): release channel domain service (channels and scopes)#1016
rl-block wants to merge 1 commit into
rollout/01b-release-channel-schemafrom
rollout/01c-release-channel-domain

Conversation

@rl-block

@rl-block rl-block commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +848/-0 across 2 files (excludes generated, test, and story files).

Summary

Adds the channel half of the rollout domain package: operators can create, edit and delete release channels, preview what a scope covers before saving, and are prevented from putting the same miner in two channels. Membership follows fleet placement, so a miner that moves into a channel's rack is a member on the next read without anyone editing the channel. The rollout engine that acts on assignments is the next PR.

Stack. #1014 (merged: API contract) -> #1015 (schema, queries, store) -> #1016 (this PR) -> #1017 (rollout engine) -> #1018 (API wiring) -> #1019 -> #1020 -> #1021 -> #1022 -> #1023. Diff is relative to #1015. Upstream context: release_channel_member (#1015) is the single source of "who is in which channel" and already resolves overlaps by selector specificity; ResolveReleaseChannelScope evaluates an unsaved scope and reports the owning channel of any miner another channel already covers; LockReleaseChannelScopes is a per-org, transaction-scoped advisory lock. Out of scope: ApplyFirmware / RollbackFirmware, rollouts and the enforcement loop (#1017); exposing any of this over RPC (#1018).

How it works

Creating or editing a channel runs in one transaction: take the org's scope lock, resolve the requested scope with PreviewScope, fail with FailedPrecondition naming the offending channels and miner counts if anything is already claimed (the channel being edited is excluded from that check), then insert or update the channel row and replace its target rows. Miner identifiers in the scope are resolved to device ids up front and an unknown identifier is an InvalidArgument. Names are trimmed, must be non-empty, and a unique-violation from the database becomes "a release channel named X already exists". The lock is what makes two operators saving overlapping scopes at the same time serialize instead of both succeeding.

Behaviour validation (Behavior.validate) fills defaults (all_at_once, least_efficient_first), rejects unknown methods and orders, requires a batch size for batched and a pilot size for pilot-then-continue, range-checks thresholds (hashrate drop 0–100 %, the rest non-negative), and normalizes away knobs the chosen method cannot use (e.g. an all-at-once channel drops batch size, review, auto-continue and thresholds; a pilot channel always reviews after the pilot). What is stored is therefore exactly what the engine will read.

Reading channels loads the org's targets, members, assignments and active rollouts in four queries and assembles Channel views grouped by model: each ModelGroup is a summary — the assigned firmware (if any), MinerCount, OnTargetCount (members reporting the assigned version), the sorted ReportedVersions, and the id of the active rollout for that model. Members themselves are read through ListChannelMiners, which returns one page (default 100, max 1000) of a channel's miners ordered by identifier, optionally one model, each with its reported version and a Conflicted flag; the opaque cursor is the last row's (identifier, id).

Previewing a scope returns miners per model and a list of ScopeConflict{ChannelID, ChannelName, MinerCount}; the same function backs both the UI preview and the write-time rejection so they cannot disagree.

Diagrams

flowchart TD
    Op["CreateChannel / UpdateChannel (spec)"] --> V["spec.validate: trim name, normalize scope, Behavior.validate"]
    V --> Tx["RunInTx"]
    Tx --> L["LockReleaseChannelScopes (org advisory lock)"]
    L --> P["PreviewScope via ResolveReleaseChannelScope (exclude self)"]
    P -->|conflicts| E["FailedPrecondition: scope overlaps release channel X (n miners)"]
    P -->|clean| W["insert / update release_channel"]
    W --> T["replaceTargets: resolve identifiers to device ids, rewrite release_channel_target"]
    T --> G["GetChannel: targets + members + firmware + active rollouts, grouped by model"]
Loading

Areas of the code involved

Area / file What changed Why it matters for review
server/internal/domain/rollout/service.go (new) Service and its dependencies (store, transactor, command dispatcher, firmware files, activity log); view types Channel, ModelGroup (summary counts), ChannelMiner, ScopePreview, ScopeConflict; CreateChannel, UpdateChannel, DeleteChannel, ListChannels, GetChannel, ListChannelMiners, PreviewScope, rejectOverlap, replaceTargets, buildChannels; shared paging helpers (clampPageSize, encodeCursor / decodeCursor, DefaultPageSize = 100, MaxPageSize = 1000) The exclusivity rule and its locking. Check that every write path takes the lock before previewing and that excludeChannelID is threaded through on update
server/internal/domain/rollout/behavior.go (new) Scope (normalize, IsEmpty, target rows), Thresholds, Behavior with validate and gatesAfterBatch, behaviorFromChannel, null helpers Validation and normalization rules; compare against the limits declared in the proto (#1014)
server/internal/domain/rollout/channel_test.go (new) Scope resolution through rack, building and site placement; overlap rejection naming the channel; self-edit allowed; unknown miner rejected; dynamic membership and Conflicted flag; duplicate name; delete; behaviour validation and normalization Test-only
server/internal/domain/rollout/fixture_test.go (new) Per-test database via dbtest, fakes for dispatcher / firmware files / activity, placement helpers (addSite, addRack, placeInSet, placeAtSite, …) Test-only; extended by #1017

Key technical decisions & trade-offs

  • Exclusivity is enforced at write time under an org-level advisory lock rather than with a database constraint, because membership is derived from placement and cannot be expressed as a unique index. The lock is transaction-scoped and per org, so it does not serialize unrelated orgs.
  • Runtime overlaps are tolerated and flagged, not fixed. A miner moved into a second channel's rack after both were saved is shown as Conflicted on the winning (more specific) channel; auto-resolving by editing scopes behind the operator's back was rejected.
  • PreviewScope is the single overlap oracle for both the UI and the write path, so what the operator sees before saving is what the server enforces.
  • Behaviour is normalized on save, not just validated, so the engine never has to reason about knobs that do not apply to a method.
  • Service already takes the command dispatcher and firmware-files dependencies its constructor will need in feat(rollout): firmware rollout engine #1017, so wiring (feat(rollout): expose RolloutService and run the enforcement loop #1018) does not change when the engine lands.
  • Channels are read with four org-wide queries and grouped in memory rather than per-channel queries, avoiding N+1 on the channels table.
  • Model groups are summaries; members are paged. buildChannels still walks the org's members once to compute counts, but the API never embeds a miner list in a channel, so response size is bounded by the number of models, not miners.
  • One cursor codec for every paged list (opaque base64 of the sort key), introduced here and reused by the rollout engine's lists in feat(rollout): firmware rollout engine #1017.

Contract alignment (after the #1014 merge)

#1014 merged on 2026-09-08 as 74454ed9. This stack was cut against the contract as it stood on 2026-09-04 (4256f05f); the contract then absorbed thirty Codex review rounds before merging. On 2026-09-08 the branch was rebased onto main so it carries only its own commit. It compiles and its tests and lint pass on the rebased head.

This PR implements: Create/update/delete with advisory-locked overlap rejection, scope preview, model-group summaries and paged channel miners.

Contract surface added after the stack was cut, not in this PR:

  • Pair keys. Assignments by canonical (manufacturer, model) with ASCII-fold uniqueness (RequiredManufacturerModelTargetKey in the contract tests).
  • Conflicts. Winner/loser/excluded-tie relations and tie exclusion (see feat(rollout): release channel schema, queries and store #1015).
  • Model groups. ListReleaseChannelModelGroups is its own paged RPC carrying reported_version_count, on_target_count by provenance, assignment_generation, firmware_checksum and firmware_available.
  • Preview. model_count/conflict_count with 100-item truncation.

These gaps are tracked in the stack status note and are the subject of the reconciliation plan for the next revision of this stack.

Testing & validation

  • DB_PASSWORD=fleet go test ./internal/domain/rollout/: 2 integration tests against a real Postgres/TimescaleDB (dbtest template clone), now also covering model-group summaries and ListChannelMiners paging, model filter, conflict flag, bad cursor and unknown channel; plus the contract validation tests from feat(rollout): RolloutService API contract for firmware release channels #1014.
  • golangci-lint run ./internal/domain/rollout/... clean.
  • Not covered here: concurrency of the advisory lock is asserted by design, not by a two-goroutine test; rollouts and enforcement are tested in feat(rollout): firmware rollout engine #1017.

@github-actions

github-actions Bot commented Sep 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 (fe0c80870613dc6960c28923b07fd4e769b96443...4ae13c69d047519c59a787b6689b8c9894eaa079, exact PR three-dot diff)
  • Model: gpt-5.6-sol

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


Review Summary

Overall Risk: HIGH

Findings

[HIGH] Automated review incomplete

  • Category: Other
  • Description: The automated review produced no usable result for fe0c80870613dc6960c28923b07fd4e769b96443...4ae13c69d047519c59a787b6689b8c9894eaa079 (workflow run 34181982977; reason: codex-job-timeout, elapsed: unknown, budget: 9 minutes).
  • Impact: The pull request has not received complete automated security, correctness, and reliability analysis.
  • Recommendation: Require human review before merging. Do not treat this result as approval-free or low risk.

Notes

Human review is required because the bounded automated review was incomplete.


Generated by Codex Security Review |
Triggered by: @rl-block |
Review workflow run

@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from abd73ab to fa30a1c Compare September 4, 2026 09:39
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from fa30a1c to 4d90bfe Compare September 4, 2026 10:10
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch 2 times, most recently from 3a7d96c to 3f03628 Compare September 4, 2026 10:36
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from 3f03628 to b3bb4d7 Compare September 4, 2026 12:06
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from b3bb4d7 to 0463a93 Compare September 8, 2026 02:32
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from 0463a93 to 4ae13c6 Compare September 8, 2026 03:00
Channel CRUD under a per-org advisory lock with overlap rejection, scope
preview with truncated model and conflict lists plus totals, and the paged
reads the contract defines: channel miners filtered by observed identity,
manufacturer/model groups joined to their assignment (checksum, snapshotted
target keys, generation, resolved file id and availability, on-target count
by provenance, first ten reported versions with the total), and membership
conflict relations with winner/loser/excluded-tie resolution. Pair keys are
canonical printable ASCII compared with an ASCII-only fold, matching the
schema and the files service. Behavior carries the coverage percent and the
delegated-control timeout; the delegated method is rejected until its slice
lands.
@rl-block
rl-block force-pushed the rollout/01c-release-channel-domain branch from 4ae13c6 to fd1c5e8 Compare September 8, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-policy: needs-review Managed by the Review Policy workflow. server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant