Drain::upload_blocks opens on:
// Resolved before any byte moves: a session that cannot authenticate the
// member's placement choice holds its content ops rather than picking a
// destination for them.
let placement = self.placement.as_ref().map_err(|_| Halt::UploadAttempt)?;
The comment says "holds its content ops". Halt::UploadAttempt does not hold: it charges the attempt budget, and at five it dead-letters through Drain::dead_letter, which retires the registry rows and releases the version's staged blocks. Comment and code say opposite things.
This is the same defect #1090 fixed for the four validate_byo_config verdicts, which PR #1320 routed to a new Halt::HeldBySettings — a hold whose exit is a settings change rather than a timer. PlacementRefusal (crates/engine/src/settings.rs) is the adjacent class and was left alone there, deliberately, because its three variants do not all want the same treatment:
NoProvider and NoExternalIngress(kind) are deterministic and settings-fixable — exactly the HeldBySettings shape. Both are gated by publish_settings, which refuses to publish settings no reader could place under, so reaching them takes a settings record written by another build or a validator change; the same defence-in-depth position the four BYO verdicts sit in.
SettingsUnavailable(reason) is not deterministic. It is a degraded settings load — a resolve that failed or timed out with no last-known-good copy — and it repairs itself on a later tick. Holding it until "the settings change" would park the queue head on a condition no member action clears. It wants the retry it has, or Halt::Unclassified.
So the fix is not a single re-map: the three variants split across two verdicts, and the split is the work.
Acceptance criteria
Note for the implementer: PR #1320's SettingsHold carries a ProviderError. Covering PlacementRefusal needs the hold's reason to widen — either a small union of the two, or PlacementRefusal gaining a Byo(ProviderError) variant with placement_of running validate_byo_config, which would collapse both sets onto one reason type and let the hold read its exit straight off the placement decision. The second shape also changes SettingsPublishError's surface, so it is not free.
Found by the /simplify altitude pass on PR #1320.
Part of #655
Drain::upload_blocksopens on:The comment says "holds its content ops".
Halt::UploadAttemptdoes not hold: it charges the attempt budget, and at five it dead-letters throughDrain::dead_letter, which retires the registry rows and releases the version's staged blocks. Comment and code say opposite things.This is the same defect #1090 fixed for the four
validate_byo_configverdicts, which PR #1320 routed to a newHalt::HeldBySettings— a hold whose exit is a settings change rather than a timer.PlacementRefusal(crates/engine/src/settings.rs) is the adjacent class and was left alone there, deliberately, because its three variants do not all want the same treatment:NoProviderandNoExternalIngress(kind)are deterministic and settings-fixable — exactly theHeldBySettingsshape. Both are gated bypublish_settings, which refuses to publish settings no reader could place under, so reaching them takes a settings record written by another build or a validator change; the same defence-in-depth position the four BYO verdicts sit in.SettingsUnavailable(reason)is not deterministic. It is a degraded settings load — a resolve that failed or timed out with no last-known-good copy — and it repairs itself on a later tick. Holding it until "the settings change" would park the queue head on a condition no member action clears. It wants the retry it has, orHalt::Unclassified.So the fix is not a single re-map: the three variants split across two verdicts, and the split is the work.
Acceptance criteria
Halt::HeldBySettingspath PR fix(engine): charge or hold the drain halts no retry can shed #1320 added, so a member's staged content is not released over a placement they can still edit.SettingsUnavailablekeeps a retrying verdict, and the comment above the call site is corrected to describe whichever behaviour it ends up with.crates/engine/tests/write_plane.rscase for each side.Note for the implementer: PR #1320's
SettingsHoldcarries aProviderError. CoveringPlacementRefusalneeds the hold's reason to widen — either a small union of the two, orPlacementRefusalgaining aByo(ProviderError)variant withplacement_ofrunningvalidate_byo_config, which would collapse both sets onto one reason type and let the hold read its exit straight off the placement decision. The second shape also changesSettingsPublishError's surface, so it is not free.Found by the
/simplifyaltitude pass on PR #1320.Part of #655