backlog: recover #1245's amendment -- proxy framing, two closed connections, and the xfail test shape - #366
Merged
Merged
Conversation
… confirming test's shape Peer review before push. Three changes, none of which move the verdict. States the mechanism more precisely: the docstring equates unclaimed-ness with must_change_password, so the flag is a PROXY for "never claimed" and the equivalence holds only while it has one writer. admin_reset_password is a second writer. The durable fix is to stop inferring claimed-ness from a mutable flag any path may raise, not to special-case the reset. Closes two connections the original bound left open, both of which hold: set_password issues an UPDATE that persists the flag, and _other_enabled_admin_exists returns true for any enabled ADMINISTRATOR other than the excluded id with no expiry wait. Individually-verified links are not a verified chain; the gaps between them are invisible exactly when every link holds. What remains unread is now named rather than implied. Records that the confirming test must assert the CORRECT behaviour under a strict xfail, not reproduce the defect. A test asserting today's behaviour turns the bug into expected behaviour, so the fix reads as a regression and the green defends the defect. strict is load-bearing: it reds when the test starts passing, so the fix cannot land silently. The reason string must name the mechanism, because the line numbers will drift.
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.
Recovers an amendment to #1245 that was written before its PR merged and did not land. Docs-only:
docs/BACKLOG.md, +13 lines, 0 deletions. Original commite1adc1cb, cherry-picked onto current main.Why this PR exists -- a race I caused
I pushed #1245 at tip
e7c16805and armed auto-merge immediately. The filing session was concurrentlyamending the item on a peer review; #365 merged at 13:55:33Z on the old tip, and
e1adc1cbneverreached the remote. Verified absent from main before opening this:
xfail0 hits, "second writer"0 hits.
The lesson is mine, not theirs: arming on push means any subsequent amendment loses the race. For a
docs-only filing with nothing downstream waiting, the arm can wait for the author to say they are
finished.
What was lost, and why it is worth a second PR
The mechanism is a PROXY defect, and naming it that way points the fix somewhere else. The
docstring equates unclaimed-ness with
must_change_password. That equivalence holds only while theflag has ONE writer -- and
admin_reset_passwordis a second. So the fix is "stop inferringclaimed-ness from a mutable flag", not "special-case the reset".
Two of the original bound's open connections are now read, and both hold --
store.py:7714-7729persists the flag via a real
UPDATE, and_other_enabled_admin_existsreturns true for any enabledadministrator other than the excluded id with no expiry wait. The remaining unverified span is smaller
and named.
The confirming test must assert the CORRECT behaviour under
@pytest.mark.xfail(strict=True). Thisis the limb most worth recovering. A test written to assert what the code does today turns the bug
into expected behaviour: the green then actively defends the defect, and whoever fixes it sees a red
and concludes they broke something.
strict=Trueis load-bearing -- it reds when the test startspassing, so the fix cannot land silently.
Verification
e1adc1cb; docs-only,+13 -0closed 74 -- unchanged in all three. An amendment adds no item and moves no status; a prose
insertion that shifts the open count is the tell that a glyph was read as a banner.
parse_items: #1245 still OPEN with exactly one status bannerThat count check is not ceremony here. The filing session hit exactly this: a no-entry glyph used as
emphasis inside an item body was read by
parse_itemsas a status banner, flipping #1245 toCLOSED -- a live defect filed as resolved, silently and semantically inverted. Their open count went
200 to 199 on a pure prose insertion, which is how they caught it.