fix(C-135): the FAO delivery is armed on a build whose upload check fails open - #381
Merged
Merged
Conversation
…dy has not views-postprocessing reviewed #380 and flagged that the pin lacks their C-79. Verified against their tree, and the finding is larger than the pin I chose. Until views-postprocessing#222 the upload verification read `if success is False: raise`. That FAILS OPEN: a result that is None, lacks the attribute, or carries a non-bool passes as though the upload succeeded, leaving an ORPHAN -- a file in the partner bucket with no metadata document. Both consumer APIs select on metadata, so the partner sees nothing while the run reports success and exits 0. Their fix is `if success is not True`. un_fao is LIVE and pinned to @main, which lacks it. That is a current gap on the UN-facing delivery, not a hypothetical about un_crafd. It cannot be fixed here. The fix is on their `development` (2eb29f1), not on `main` (3286eab), and `main` is the newest MERGED state carrying the crafd package at all -- the only tag, 1.0.0, contains zero crafd files. So no merged pin exists today with both crafd and C-79. Moving a live delivery to an unmerged branch is a worse trade than waiting for their sync, so this is registered and guarded rather than patched. un_crafd is not exposed: paused means the manager never constructs a partner store and makes zero store calls. The gap needs an upload to reach it. So the disarmed launcher on this pin is safe, and the danger is precisely the transition -- which is what the guard is about. tests/test_launcher_pin_safety.py: test_a_disarmed_launcher_stays_disarmed_while_its_pin_is_deficient Passes for a disarmed launcher on any pin, and for an armed launcher on a good pin. Fails only on the combination that ships orphans. Verified: arming un_crafd without moving the pin turns it red and names why. test_no_armed_launcher_is_pinned_to_a_deficient_build The state we want, recorded as a STRICT xfail rather than omitted. It documents un_fao's live gap, and flips to XPASS -- so fails, forcing the marker's removal -- the day the pin moves. The exit is upstream and is one action: merge development -> main, which is also what lets them cut the tag #364 asks for. That issue now blocks two repositories.
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
1 participant
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.
views-postprocessing reviewed #380 and flagged that the
un_crafdpin lacks their C-79. Verified against their tree — and the finding is larger than the pin I chose.The gap
Until views-postprocessing#222 the upload verification read
if success is False: raise. That fails open: a result that isNone, lacks the attribute, or carries a non-bool passes as though the upload succeeded — leaving an orphan, a file in the partner bucket with no metadata document. Both consumer APIs select on metadata, so the partner sees nothing while the run reports success and exits 0. Their fix isif success is not True.un_faoisliveand pinned to@main, which lacks it. That is a current gap on the UN-facing delivery, not a hypothetical aboutun_crafd.Why it is registered rather than patched
The fix is on views-postprocessing's
development(2eb29f1), not onmain(3286eab).mainis the newest merged state carrying the crafd package at all — the only tag,1.0.0, contains zero crafd files.So no merged pin exists today with both crafd and C-79. Moving a live delivery's pin to an unmerged branch is a worse trade than waiting for their sync.
un_crafdis not exposed:pausedmeans the manager never constructs a partner store and makes zero store calls. The gap needs an upload to reach it. The disarmed launcher on this pin is safe; the danger is precisely the transition.The guard
tests/test_launcher_pin_safety.py:test_a_disarmed_launcher_stays_disarmed_while_its_pin_is_deficient— passes for a disarmed launcher on any pin, and for an armed launcher on a good pin. Fails only on the combination that ships orphans. Verified: armingun_crafdwithout moving the pin turns it red and names why.test_no_armed_launcher_is_pinned_to_a_deficient_build— the state we want, recorded as a strict xfail rather than omitted. It documentsun_fao's live gap and flips to XPASS — so fails, forcing the marker's removal — the day the pin moves.The exit is upstream, and it is one action
views-postprocessing merges
development→main. That is also what lets them cut the tag #364 asks for, which now blocks two repositories.Registered as C-135 (Tier 2, Open).
Verification
ruff check .clean; full suite 7796 passed, 0 failed (7 xfailed — one new, deliberate)