Repository navigation
Make the checkout part of the publication contract - #39
Merged
Merged
Conversation
`sha256sum -c docs/ui-design/SHA256SUMS` did not verify in a Windows clone. Git checks text out as CRLF there under the default core.autocrlf=true, the recorded digests are of the committed LF form, and every frozen artifact reported as modified in a tree nobody had touched. A checkout artefact reported as tampering is the worst way for an integrity gate to fail: loud, wrong, and it teaches the reader to distrust the gate rather than the tree. #37 worked around this one layer up, folding line endings in the test before comparing. That kept the suite honest and left the plain shell command still wrong, which is the half that a person checking the baseline from a clone actually runs. This closes it where it happens: * text=auto eol=lf *.png binary `eol=lf` is the load-bearing half. `text=auto` alone normalises what git stores and then defers to the client's core.autocrlf for what it writes out -- exactly the case that broke the check. The PNG rule changes nothing today, since git finds a NUL byte in every PNG and classifies them correctly already; it is there so that stays a rule rather than a heuristic. Adding the file made it an unowned root file, so verify_tree owns it, with its reason recorded beside it: this is a publication guarantee, not an editor preference. 98 -> 99 publishable text files. The test that matters runs real git rather than simulating it, because the claim is about what git writes. It commits the frozen artifacts into a fresh repository under core.autocrlf=true, deletes them, checks them out again, and compares against the digests in SHA256SUMS. Its control is the point: the same round trip with no .gitattributes must produce bytes that do NOT verify, or this platform is not reproducing the situation and the passing half proves nothing about it. The PNG survives both, which is why that rule is insurance. Mutations, each alone: eol=lf dropped, text=auto kept FAIL (declaration and behaviour) the *.png rule removed FAIL .gitattributes removed FAIL (behaviour and ownership) the gate stops owning it FAIL, and verify_tree reports "Unowned root file: .gitattributes" The first is the plausible half-fix, and it fails the behavioural test rather than only the one that reads the file. Nothing was renormalised: all 98 tracked text files were already LF in the index and the 6 binaries were already -text, so no recorded digest moves. Verified after the change, from docs/ui-design/, that `sha256sum -c SHA256SUMS` reports OK for all eight. Gates: 1676 -> 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. docs/ui-design/ untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
Windows 3.13 failed on this head, in a test this PR does not touch: test_a_command_needs_a_length_it_can_be_held_to ConnectionAbortedError: [WinError 10053] The cause is already documented in this repository, in Client.unread()'s docstring and again in #36's second commit: a surface that settles a refusal without touching the body leaves the sent bytes unread in the receive buffer, and closing on unread bytes is a reset rather than a clean shutdown on Windows. Whether the refusal or the reset reaches the client first is a race. So this is a latent defect on main that this head surfaced, not one it introduced -- and the fix already exists here. #36 removed the same race from the intake client by declaring a length and sending nothing. This ports `transmit` to the surface client and tags the cases that qualify. It also states each claim more exactly. The request that proves the server refused without reading is the request whose body never arrived, and "refused on what it declares" is a stronger sentence when the oversized body demonstrably never travelled. Tagged, all verified to answer before the body is reached: the missing Content-Length (the failure) the media type, refusing loop (not the charset case, which is a 200) the declared oversize, command the declared oversize, decision the query string Deliberately not tagged: test_a_body_that_is_not_a_json_object_is_refused -- the server must read that body to refuse it, so withholding it would hang; test_the_command_refuses_before_reading_the_body_in_the_order_that_- matters -- its last assertion requires the body to be read, and its stated method is to send a body that would itself be a 400. Rewriting that proof is more than porting a flag, so it is left for a decision rather than changed in passing. The distinction is mechanical rather than judged. Mis-tagging a case whose body IS read does not pass quietly -- it waits for the client timeout. Mis-tagging test_a_body_that_is_not_a_json_object_is_refused on purpose: 6 failed in 61s, every one of them a timeout. The five tagged here run in 9s with no timeout, which is the evidence they qualify. That check is the one #36 learned by getting it wrong once. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
The matrix went green on dcbdd1f and I could not establish, from the logs, that the test which proves the whole point of this PR had actually run on Windows rather than skipping. Checking took three log fetches and still did not reach the pytest summary line, which is its own argument: a claim that costs that much to verify is one nobody will re-verify. A skip reads as a pass. On the one platform this contract exists for, a green cell that silently covered nothing is worse than a red one -- it is the same failure as the screenshot attestation two PRs ago, where the test passed while proving something other than what it claimed. So git is required instead of detected. That is honest rather than strict: these tests ship only in the source tree, which is obtained by cloning, so an environment running them without git is one that cannot have this repository in the first place. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
Review is right, and the old test proved the weaker property. It sent the malformed body on every request, which establishes only that the bytes were not PARSED before Host, Origin and token were checked. A server that read the whole body into memory and then checked provenance would have passed it unchanged. The three refusals now declare a body and send none. Each must still answer, which it can only do without reading -- a server reaching for those bytes waits until the client times out. The fourth request sends the same body with provenance in order and must read it to find the malformed JSON, so the pair separates "refused without reading" from "refused after reading" rather than assuming the difference. Demonstrated against a server mutated to buffer the whole body before any provenance check -- the exact design the old test could not see: old test (body transmitted every time) PASS in 0.35s new test (three refusals withhold it) FAIL, 10s timeout The first attempt at that mutation was not faithful and I nearly recorded its result. A bare read-and-discard made the OLD test fail too, but for an artifact rather than the property: the body was consumed, so the fourth request blocked re-reading it. Buffering the bytes so later reads still see them is what isolates the ordering claim, and only then does the old test pass while the new one fails. An earlier attempt was wrong in a different way and also caught: the mutation went into _resolve(), which is the GET path, so it changed nothing a POST test could observe and the suite passed. _command() is where a command settles provenance. This also removes the last instance of the unread-body reset race from this file, which dcbdd1f left behind deliberately because rewriting a load-bearing proof is not porting a flag. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
Deep-Sixed
marked this pull request as ready for review
September 20, 2026 05:11
Deep-Sixed
added a commit
that referenced
this pull request
Sep 22, 2026
The Windows 3.12 cell failed on test_an_outward_command_refuses_an_unknown _query_parameter with WinError 10053, which is the unread-body reset race #39 was written to remove: `_outward` settles accepted() on the query before it reaches for _body(), so a declared body goes unread, and closing on unread bytes is a reset rather than a clean shutdown on Windows. Whether the 400 or the reset reaches the client first is then a coin toss. #39 fixed five call sites and claimed in its own docstring to have removed "the last instance ... from this file". That claim was wrong when I wrote it. This call site arrived in #35 and I missed it; the sweep behind #39 was not a sweep. An AST pass over all 112 client call sites in the file now says this was the only one left: every other send carrying a body either declares transmit=False already or is an address that genuinely reads -- malformed JSON, the charset 200, and the provenance-in-order half of the ordering test -- and the 39 calls through outward()/decide() never override provenance, so they always reach the read. The whole-file count in that docstring is replaced by the mechanism, which cannot rot the same way: the file's rule is that an address refusing before reading declares its body and sends none, written down on Client.send. Withholding is the stronger fix rather than tolerating the race with unread(), and it turns out to strengthen the test itself: drop accepted() from _outward FAIL (TimeoutError) check the query AFTER _body() FAIL (TimeoutError) the same reorder, body transmitted PASS <- what the old form allowed The third line is the point. With the body sent, reading first and judging the query second still answers 400, so the old assertion held while the ordering it is named for had been inverted. Withheld, the refusal can only arrive if the query was judged first, so the test now proves its own title instead of a status code. Gates on this commit: 1686 tests, ruff check and format clean, verify_tree 100 publishable text files / 5 reviewed renders / 1 reviewed screenshot, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
Deep-Sixed
added a commit
that referenced
this pull request
Sep 23, 2026
* Open a 0003-era database with the CareerSignal that exists now Seven migrations, and every test of them starts from nothing: create a fresh database, apply all seven, assert the result. That proves the SQL executes. It says nothing about the thing an operator depends on, which is that a database holding months of their decisions still means what it meant after the upgrade runs over it. So this builds a genuine 0003-era database -- the real historical migration resources, applied by the real runner with the file list narrowed, carrying rows written the way that schema allowed -- and lets the current code open it. No fixture .db is committed: a checked-in binary is one nobody reviews, and it would drift from the migrations it claims to predate. What is asserted is semantic rather than structural: old facts survive an opportunity recorded then reads now 0004 backfills honestly the row says a migration wrote it 0005 invents no authority an old approval authorizes no draft 0006 invents no target no addressing appears for one 0007 keeps one-attempt an old intent still bars a second The three "invents nothing" claims are why this exists. 0005, 0006 and 0007 each added columns binding an approval to what the operator read, and each deliberately left old rows unmatched, because the database has no record of what those approvals were for. A migration that backfilled a plausible value would authorize an outward write nobody approved -- and every structural test here would still pass. Mutations, each alone: 0004 backfills actor='operator' FAIL 0007 defaults provider to 'controlled' FAIL the fixture builds a CURRENT database FAIL (7 errors) the reviews share one opportunity again FAIL The third and fourth are vacuity guards, and the fourth is there because I wrote the defect it catches. The first version of this fixture pointed one opportunity's current_review at one of three reviews, so the other two were stale -- and an authorization check reads its binding through current_review, refusing a stale review before any 0005/0006/0007 column is consulted. All seven tests passed. Both outward tests were green about staleness while claiming to be about migrations. It surfaced by testing the test: I fabricated a complete, correct binding for the old approval, as a dangerous migration would, and asked whether draft() then created one. It did not -- it raised "Review is missing or stale", which is the wrong refusal and meant the test could not have caught a fabricating migration at all. With each review current for its own opportunity, the same experiment creates a real draft: provider.calls = 1, intent confirmed. That is what the semantic test asserts against, and only now does asserting it mean anything. The staleness precondition is now asserted in the vacuity test rather than left to hold by luck. Gates: 1679 -> 1686 tests, ruff clean, verify_tree 100/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo * Narrow the two outward-safety refusals to the exact exception Review is right, and the old assertions proved a weaker property than the PR's own claim. `pytest.raises(Exception)` passes for the intended refusal -- Repository.claim() raising ValueError("Approval predates draft authorization binding...") or workflow.py raising ValueError("Requested provider does not match the draft intent already reserved...") -- but it would also pass, silently, if a future unrelated failure in _binding(), draft_material(), provider identity resolution, or SQLite access threw first. provider.calls == 0 and intent is None would still hold, and the regression test would stay green while proving nothing about the historical-upgrade contract it exists to guard -- the exact class of defect this file was written to catch. Both assertions now name the exact exception and match its message, so a different failure earlier in draft() fails the test instead of passing it for the wrong reason. Gates: 1686 tests (both narrowed assertions still pass against the real refusal), ruff clean, verify_tree 100/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KR2F1vgfq8mygoDtcSaSyp * Withhold the body on the one refusal that still sent it unread The Windows 3.12 cell failed on test_an_outward_command_refuses_an_unknown _query_parameter with WinError 10053, which is the unread-body reset race #39 was written to remove: `_outward` settles accepted() on the query before it reaches for _body(), so a declared body goes unread, and closing on unread bytes is a reset rather than a clean shutdown on Windows. Whether the 400 or the reset reaches the client first is then a coin toss. #39 fixed five call sites and claimed in its own docstring to have removed "the last instance ... from this file". That claim was wrong when I wrote it. This call site arrived in #35 and I missed it; the sweep behind #39 was not a sweep. An AST pass over all 112 client call sites in the file now says this was the only one left: every other send carrying a body either declares transmit=False already or is an address that genuinely reads -- malformed JSON, the charset 200, and the provenance-in-order half of the ordering test -- and the 39 calls through outward()/decide() never override provenance, so they always reach the read. The whole-file count in that docstring is replaced by the mechanism, which cannot rot the same way: the file's rule is that an address refusing before reading declares its body and sends none, written down on Client.send. Withholding is the stronger fix rather than tolerating the race with unread(), and it turns out to strengthen the test itself: drop accepted() from _outward FAIL (TimeoutError) check the query AFTER _body() FAIL (TimeoutError) the same reorder, body transmitted PASS <- what the old form allowed The third line is the point. With the body sent, reading first and judging the query second still answers 400, so the old assertion held while the ordering it is named for had been inverted. Withheld, the refusal can only arrive if the query was judged first, so the test now proves its own title instead of a status code. Gates on this commit: 1686 tests, ruff check and format clean, verify_tree 100 publishable text files / 5 reviewed renders / 1 reviewed screenshot, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Deep-Sixed
added a commit
that referenced
this pull request
Sep 26, 2026
Two findings from the review of #36-#40, both in the intake code #36 added. A refusal decided on the envelope -- 413, 415, the namespace 400 -- is answered before a byte of the body is read, which is right, and then the socket was closed on whatever the client had sent, which is not. Closing on unread bytes is a reset, and a reset can overtake the response; on Windows it discards a response the client has not read yet. The page always sends the file it was given, so an operator choosing a 3 MB message could see "Failed to fetch" instead of the ceiling that refused it. The surface now settles every response the same way: write the answer, shut its own send side, then read and discard whatever the client is still sending, bounded at 16 MiB and five seconds, and only then close. Nothing about what is decided, or when, changes -- the answer is written before the drain begins and the drained bytes are never parsed, stored or kept. The socket is reached as `self.request`, socketserver's own name for it, because this package is mechanically forbidden the word `connection` and that guard is right to be. The suite could not see this. Every pre-read refusal was asserted with transmit=False, which proves the surface decides before reading and cannot prove what a browser sees, because a browser always sends the body. That was my choice in #39 and #40, and I did not say what it left untested. These tests send it: unfixed surface, deterministic orderly-close test FAIL 20/20 runs unfixed surface, over-ceiling full-body test FAIL 10/10, then 1 pass in 3 drain with no time bound FAIL (thread held) drain without shutting the send side first FAIL (no early end-of-stream) The small-body 415 and 400 full-body cases never fail on Linux; they are there for Windows, where the reset destroys the response rather than trailing it. `gmail-ingest --limit 0` reported "the mailbox could not be read". The service checked label_ids before its try and let limit fall inside it, where every failure becomes SourceUnreadable -- and only after the identity read had spent the credential. The limit is now checked where label_ids is, against the reader's own rule, and the CLI refuses a non-positive --limit before Gmail or the database is touched. service limit check removed FAIL (4 cases) CLI limit check removed FAIL Finding #1 of the review, a malformed read token stopping serve, is not in this change: whether that launch should refuse or start without Gmail is a decision still open. Gates: 1686 -> 1696 tests, ruff check and format clean, verify_tree 100/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo
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
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.
Closes the carried
.gitattributesitem, which the career-ops comparison moved from cleanup to correctness — plus two test-integrity fixes that the work surfaced along the way.Head:
44c831eBase:
main @ 3dd222cWhat is in here, and why four files changed
Somebody reading only the title would expect one file. Two of the four are
test_web_surface.pywork that this PR did not set out to do:86f8e56.gitattributes, its ownership, its testsdcbdd1ftest_web_surface.py, surfaced by this PR's CI, fixed as a class2aa7c0b44c831e1. The checkout contract
sha256sum -c docs/ui-design/SHA256SUMSdid not verify in a Windows clone. Git checks text out as CRLF there under the defaultcore.autocrlf=true, the recorded digests are of the committed LF form, and every frozen artifact reported as modified in a tree nobody had touched.A checkout artefact reported as tampering is the worst way for an integrity gate to fail: loud, wrong, and it teaches the reader to distrust the gate rather than the tree.
#37 worked around this one layer up, folding line endings in
as_committed()before comparing. That kept the suite honest and left the plain shell command still wrong — the half a person checking the baseline from a clone actually runs.eol=lfis the load-bearing half:text=autoalone normalises what git stores and then defers to the client'score.autocrlffor what it writes out. The PNG rule changes nothing today — git finds a NUL byte in every PNG — and is there so that stays a rule rather than a heuristic.verify_tree.pyowns the file, with its reason recorded beside the entry. 98 → 99 publishable text files.Nothing was renormalised. All 98 tracked text files were already LF in the index and all 6 binaries already
-text, so no recorded digest can move. Confirmed after the change, fromdocs/ui-design/:sha256sum -c SHA256SUMSreports OK for all eight.The test runs real git, since the claim is about what git writes. Fresh repo under
core.autocrlf=true, commit, delete, check out, compare againstSHA256SUMS. Its control is the point: the same round trip without.gitattributesmust produce bytes that do not verify, or the platform is not reproducing the situation and the passing half proves nothing.2. A pre-existing Windows race (
dcbdd1f)Windows 3.13 failed on
86f8e56intest_a_command_needs_a_length_it_can_be_held_to— a test this PR does not touch — withConnectionAbortedError: [WinError 10053]. When the surface settles a refusal without touching the body, the sent bytes sit unread in the receive buffer, and closing on unread bytes is a reset rather than a clean shutdown on Windows.Latent on main, surfaced here. The fix already existed in this repo: #36 removed the identical race from the intake client with
transmit=False. This ports that flag to the surface client and tags the five cases that qualify.The distinction is mechanical, not judged: mis-tagging a case whose body is read does not pass quietly, it waits for the client timeout. Mis-tagging the JSON-parse test on purpose gave 6 failures in 61s, all timeouts; the tagged cases run in 9s with none.
3. No silent skip (
2aa7c0b)The matrix went green on
dcbdd1fand I could not establish from the logs that the checkout test had run on Windows rather than skipping. A skip reads as a pass, and on the one platform this contract exists for, a green cell covering nothing is worse than a red one. Git is now required rather than detected — honest rather than strict, since these tests ship only in the source tree, which is obtained by cloning.4. Unread, not merely unparsed (
44c831e)test_the_command_refuses_before_reading_the_body_in_the_order_that_matterstransmitted its malformed body on every request, which establishes only that the bytes were not parsed before Host/Origin/token. A server that read the whole body into memory and then checked provenance would have passed it.The three refusals now declare a body and send none; the fourth sends it and must read it to find the malformed JSON. Demonstrated against a server mutated to buffer the entire body before any provenance check:
Mutation coverage
eol=lfdropped,text=autokept*.pngrule removed.gitattributesremoved entirelyUnowned root file: .gitattributestransmit=Falsetag on a body the server readsVerification on
44c831e1679 tests, Ruff check and format clean,
verify_tree99 publishable text files / 5 reviewed renders / 1 reviewed screenshot,verify_secretsPASS,verify_wheelPASS.Previous head
2aa7c0bwas 24/24 green — push run 250 (12/12) and PR run 251 (12/12). The hosted matrix on this exact head is the merge gate.docs/ui-design/**untouched;dashboard.pnguntouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo