Skip to content

Batch 460 wave 2: upload cancel and teardown fixes, quota SQL, trusted-proxies docs (BACKLOG #3740, #3764) - #2645

Merged
wshallwshall merged 26 commits into
mainfrom
batch/b460-wave2
Oct 30, 2026
Merged

wshallwshall merged 26 commits into
mainfrom
batch/b460-wave2

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Batch 460 wave 2 is four Builder branches on batch/b460-wave2: three items for BACKLOG #3740 and one for #3764. None changes a security control. The wave's control changes ship in their own PRs: 2634, 2636, 2637, 2638 and 2640. Every item below is reversible.

Combined-tree checks (head of batch/b460-wave2)

Check Result
ruff check ., ruff format --check . clean
mypy: linux engine, win32 engine, linux tests clean
ledger_check.py --ci rc 0
changelog fragments 68 well-formed
pytest, 19 targeted files (uploads, quota SQL, lifespan teardown, outlive, TLS, ambient proxy, settings, approvals, refusal chains, tooling partition, doc drift) 1092 passed, 10 skipped

The full suite was not run. The PostgreSQL and SQL Server store legs, windows-service-smoke, the Windows pytest legs and CodeQL run hosted only, and are named per item below.


W2a: BACKLOG #3740 items 3 (reason in the code), 4, 11, 13 and part of 12

Source branch b460w2-3740-uploads-text, head f7cd6839774b4b45d7f5ad1bc67e83da31e61db5.

Merge danger

Reversible: yes.
Blast radius:

  • UploadStore.save's cancel arm in messagefoundry/uploads.py, through the new _remove_cancelled_pair, _submit_to_thread and _CANCELLED_REMOVAL_WAIT_SECONDS.
  • The drain_stragglers defaults.
  • Comments and docstrings in api/app.py, api/outlive.py, api/approvals.py and uploads.py.
  • Two changelog fragments.
  • One-line "engine shard" qualifiers in comments only: store/base.py, store/postgres.py, store/sqlserver.py, store/store.py and uploads.py. The schema hash is unchanged.

Evidence

  • Item 4. test_a_hung_removal_of_a_cancelled_pair_does_not_hold_the_quota_lock.
    • Before: it failed on the unfixed uploads.py ("B waited on the lock for A's hung unlink").
    • After: it passes.
  • Item 12. test_the_drain_reads_its_default_waits_when_it_runs.
    • Before: it timed out on the unfixed code.
    • After: it passes.

What changed

  • Item 3, kept by design. This follows the owner's delegated ruling of 2026-10-29, decision d20261029T205249596-4de5a7. The reason now sits at upload_file's outliving.run(_record_upload_create...) site. Each stored copy has its own upload.create row, is owned by its uploader and counts against that uploader's quota. Discarding on a deadline would reopen #3178.
  • Item 4. The cancelled-pair removal waits at most 5 s, then releases the lock. If the removal is still running, it logs an ERROR naming the file_id. If the removal later lands, a WARNING withdraws that ERROR.
  • Item 11. The "released by its id" wording now reads "a release is started". The outliving comment is corrected. The 0.5.2 "dropped at open" correction is a new fragment, changelog.d/3740-schema-drop-note.fixed.md, because changelog.d/README.md forbids editing CHANGELOG.md.
  • Item 13. The stale docstrings in api/outlive.py and api/approvals.py are corrected.
  • Item 12, part. drain_stragglers now reads its waits at call time. "discard could reuse _run_to_completion" was not done, because PR 2637 rewrites discard.

Open, for a new row. UploadStore.discard() goes through _discard_under_lock, which still holds _quota_lock with no bound if an unlink hangs after a failed upload.create row. The docs/PHI.md uploads row also does not list a removal abandoned at the 5 s bound. PR 2637 edits that function and that row, so this is a follow-up.

Other open findings:

  • An abandoned removal can hold a default-executor worker, with no cap.
  • A Windows sidecar unlink can be refused after the lock drops.
  • The anyio wait loop spins for up to the bound.

W2b: BACKLOG #3740 items 10 and the SQL part of 12

Source branch b460w2-3740-store-sql, head 2efc4deb75e174c624b02aa58e7cb2cf795b80d9.

Merge danger

Reversible: yes.
Blast radius:

  • release_upload_quota(*, uploader_id, reservation_id) changes on the Store protocol, the UploadQuotaLedger protocol and all three backends. It is keyword-only.
  • The DELETE on every backend is now scoped to the uploader.
  • On SQL Server, the release no longer reads the uploader under READCOMMITTEDLOCK. It takes the named uploader's applock, through a new shared opener, _open_under_upload_quota_applock.
  • On all three backends, the reserve's COUNT and SUM are one derived-table aggregate.
  • No DDL changes.

Evidence

  • Before: 8 of 8 new tests in tests/test_upload_quota_store_sql.py failed on origin/main. The aggregate counted 2 reads, and the release signature and scoping failed.
  • After: they pass. The store contract in tests/test_uploads_shard_quota_interleave.py gains a wrong-uploader release case and test_the_reserve_budget_admits_an_exact_fit_and_refuses_one_past_it, on all three backends.

What changed

  • Item 10. The read could not move under the applock, because the applock's name comes from that read. So the uploader is now passed in.
  • Round trips (correction to the item's premise): a release whose row exists still makes 4 round trips. A release with no row now makes 4 instead of 2, and can wait on a sibling's reserve, bounded by the applock timeout. What that buys: a release now waits out a reserve that holds the lock but has not inserted yet, a case the old read missed.
  • Items 7 and 8 untouched. Item 7 is kept by owner ruling. Item 8 was already fixed by _PG_EPOCH_NOW and _MSSQL_EPOCH_NOW.

Hosted legs to read: the [postgres] and [sqlserver] cases of tests/test_uploads_shard_quota_interleave.py, the only live checks of the new SQL.

Open, for a new row. The PostgreSQL release takes no per-uploader advisory lock, so on PostgreSQL a release can still miss an uncommitted reserve. This predates the branch.

Other open findings:

  • SQL Server padded string equality.
  • The upstream positional ids in uploads.py.
  • touch_upload_quota matches by reservation id only.
  • No live race test.
  • The aggregate is written six times.

W2c: BACKLOG #3740 items 6, 12 (teardown), 14 to 18, and #3819 item 1

Source branch b460w2-3740-teardown-r, head 98cdc0e5e7f38451af61c2ff440e976eed62a7eb. It was rebased by its Builder onto this batch branch, after a conflict with W2a in messagefoundry/uploads.py. The Builder resolved the conflict, and the resolution had its own two review rounds. The superseded branch b460w2-3740-teardown (a404092ad4) is not in this PR.

Merge danger

Reversible: yes.
Blast radius:

  • In api/app.py: _app_drain_steps, a new _managed_teardown_steps, and _OUTLIVING_WAIT_SHARE.
  • In api/outlive.py: a new OutlivingOperations.settle.
  • In uploads.py: drain_stragglers, drain_stragglers_within, held touches (_touches, _cancel_touches, _running) and the heartbeat's finally.
  • Teardown and upload tests, and a changelog fragment.

Evidence

  • Item 6. test_a_removal_a_cancelled_create_row_starts_late_is_drained_before_the_teardown_ends is red on the unfixed code.
  • Item 14. test_the_shutdown_drain_does_not_wait_for_a_heartbeat_touch (four cases) and test_the_drain_cancels_a_held_touch_once are red on the old code.
  • Item 16. test_the_managed_steps_are_built_in_their_pinned_order and test_the_managed_lifespan_runs_its_real_steps_in_the_pinned_order pin the order.
  • Item 17. A new parametrize case is red on the old literal 2/3.
  • After: they pass. The Builder's final run of the teardown, upload, outlive and summary-flush set gave 255 passed and 10 skipped.

What changed

  • Item 6. The docstring's claim was false, so it is corrected. settle(timeout) now waits without cancelling, within the drain's wait and never its grace. A removal can still start late if its operation outlasts that wait. The docstring says so.
  • Item 12, teardown part. Already gone at HEAD.
  • Item 14. Built.
  • Item 15. The window test counts the real bounded steps. The total is 10.0 s budget + 3.0 s uvicorn drain + 10 x 0.1 s cut grace = 14.0 s. With the 1.0 s exit room that is exactly the 15.0 s window, so a first deployment would have zero headroom. No budget constant moved. This is a note for #3819.
  • Item 16. Built.
  • Item 17, half met. The straggler split is derived. The long-operations split stays named as _OUTLIVING_WAIT_SHARE: deriving it would cut the wait from 0.5 s to about 0.21 s and cancel work that would otherwise finish. Its derivation half stays OPEN on the row.
  • Item 18. Two over-claiming test docstrings are corrected. This is a reading: PR 2362's comment does not name the lines.

Review coverage: the last commit, 98cdc0e5e7, which cancels touches before settle, came after the two-round cap. No review round covers it.

Hosted legs to read: windows-service-smoke and the Windows pytest legs. The new tests are timing-based, with 1 to 2 s bounds.

Open findings:

  • A cut that lands during the follow-on drain gets no cancel-all.
  • A late removal is now held and then cancelled at about 0 s.
  • cancelling() > 0 is read as "already cancelled by us".
  • The 60 s cap timer under the embedded lifespan.
  • The leftover-wait WARNING text.
  • A zero-division if both defaults are 0.
  • settle waits on every long operation.
  • Touches are still held in _stragglers.
  • The settle test has no lower bound.

W2d: BACKLOG #3764 items 1 (docs, by ruling), 4, 5 (confirmed met), 7 to 10, 12 to 15

Source branch b460w2-3764-hygiene, head eec940cdb3ba249a2f07ebdff90d8fb74d800f3d.

Merge danger

Reversible: yes.
Blast radius:

  • The trusted-proxies refusal advice in config/settings.py (_ZONED_HOP_CAVEAT).
  • The trusted_proxies row of docs/CONFIGURATION.md.
  • ADR 0151, with a dated amendment, and its index row.
  • In __main__.py, the serve info log line, the uvicorn_trusted_hosts import and its call site.
  • tests/test_api_tls.py and tests/test_ambient_proxy_disclosure.py, and a changelog fragment.

Evidence

  • Item 1. A test pins uvicorn's behaviour: ('fe80::1%eth0', 0), a zoneless control, and the /128 measurement. A parametrized refusal test covers four entries, including newline injection, plus a control. Both are red on the unfixed code.
  • After: they pass. About 2,200 tests passed across the affected files.

What changed

  • Item 1, by the owner's delegated ruling (decision d20261029T205256184-f14632). A zoned hop written by an inner proxy will not match the zoneless entry, and becomes the client address. The refusal advice and the docs now say so, and name what that costs: the audit, the per-IP login limiter and allowed_client_networks all see the zoned address. There is no code match.
  • Item 1, /128 (decision d20261029T223607035-82fdc4). The refusal does not advise <host>/128. That would match a zoned hop on any zone. The behaviour is pinned by a test only.
  • Item 4. ADR 0151 D-1 is corrected with a dated amendment. No new ADR number.
  • Item 5. Confirmed already met.
  • Items 7 and 9. The log line says what the code does, and is wrapped under 100 columns.
  • Items 8 and 13. The duplicate import is folded. The call moves into run_kwargs, with the real reason in the comment: the settings load already refuses a bad entry.
  • Item 12. The test comment is corrected.
  • Items 14 and 15. The test reuses _kept and reads after redaction.

Item 10 / 11, for the Lander or the owner. The edit touches the format string of the call that CodeQL alert 209 flags. The alert will likely need re-dismissal after merge, under ADR 0034's amendment rule 3. That is item 11, a repository act.

Not built: item 6, which needs a Linux host with uvloop.

Open findings:

  • docs/adr/0150 around line 122 still says forwarded_allow_ips = [api].trusted_proxies.
  • A third _Keep copy remains in tests/test_header_floor_wire.py.

Ledger asks for the Lander

  • Flip notes on #3740 (W2a, W2b, W2c) and #3764 (W2d), using the proposed banner in each source branch's final commit body.
  • #3740 items 3, 5 and 7, and #3764 item 1, carry the owner-delegated rulings already posted on PRs 2637 and 2638.
  • Item 17's derivation half stays open.
  • New rows:
    • the unbounded discard() path, plus the PHI.md abandoned-removal entry;
    • the PostgreSQL release advisory lock;
    • the NSSM zero-headroom numbers, as a note on #3819 rather than a new row.

Landing order: none. Expect textual overlaps only, with PRs 2637 (uploads.py, api/app.py), 2641 (__main__.py, docs/CONFIGURATION.md) and 2640 (store files).

…y log line says what it trusts

The refusal of a zoned [api].trusted_proxies host now adds, after its
"List 'fe80::1' instead." advice, that a hop an inner proxy writes into
X-Forwarded-For with its zone will not match that entry and becomes the
client address. That fails closed. A test pins the uvicorn behaviour and
the message. No zone matching is built (owner's delegated ruling).

serve: the TLS-terminating-proxy log line says which entries gain a twin
spelling and fits 100 columns; uvicorn_trusted_hosts joins the settings
import and is called where its result is used, with the reason it cannot
refuse there. test_api_tls reuses _kept from test_ambient_proxy_disclosure,
reads each message at the end, and corrects the CodeQL comment.

BACKLOG #3764 items 1, 7, 8, 9, 12, 13, 14 and 15.
…ates the zoned-hop gap

D-1 said __main__.py feeds forwarded_allow_ips verbatim from
[api].trusted_proxies. It is uvicorn_trusted_hosts' result: the entries as
written plus each IPv4 or IPv4-mapped entry's other spelling. Corrected
inline, with a dated amendment section and an index note. The
trusted_proxies row of CONFIGURATION.md gains the zoned-hop caveat.

Item 5 was already met: docs/SECURITY-LOOSENING.md says an IPv4-mapped
entry counts as the IPv4 range it names. Items 6 (a Linux uvloop
IPV6_V6ONLY measurement) and 11 (the CodeQL alert 209 dismissal) stay
open. Alert 209 will likely need re-dismissal after merge.

BACKLOG #3764 items 1, 4, 5 (confirmed met), 7, 8, 9, 10 (reported), 12, 13, 14 and 15.
Proposed PR title: Trusted-proxy hygiene: the zoned-hop gap is stated beside its refusal, ADR 0151 D-1 says what uvicorn is handed
Proposed banner: Items 1, 4, 7, 8, 9, 12, 13, 14 and 15 built; item 5 confirmed already met; item 10 reported (alert 209 would need re-dismissal after merge). Open: item 6 (a uvloop IPV6_V6ONLY measurement on a Linux host), item 11 (the CodeQL alert 209 dismissal, a repository act), and folding a zone off an XFF hop, a follow-up. Items 2 and 3 ride PRs 2638 and 2634.
… in one aggregate

Store.release_upload_quota and UploadQuotaLedger.release_upload_quota
take (uploader_id, reservation_id). On every backend the delete
matches both, so a release cannot remove another uploader's row.

SQL Server: the release no longer reads the row's uploader under
READCOMMITTEDLOCK to name its applock. It takes the named uploader's
applock unconditionally, opened by the same statement the reserve
uses (_open_under_upload_quota_applock), so its lock order no longer
rests on that hint. Round trips are unchanged: the applock still
needs a table-touching opener to begin the implicit transaction.

The reserve's in-flight COUNT and SUM, two subqueries over the same
rows, are one derived-table aggregate on SQLite, PostgreSQL and SQL
Server, with the same budget decision.

BACKLOG #3740 items 10 and 12 (item 12: the SQL part only).

Proposed PR title: batch 460 wave 2 (BACKLOG #3740, #3764)
Proposed banner: Item 10 built: the upload-quota release takes the uploader on all three backends, and the SQL Server release takes that uploader's applock with no read to learn it; the transaction opener stays, so its round trips are unchanged. Item 12's SQL part built: the reserve's COUNT and SUM are one aggregate on every backend. The PostgreSQL and SQL Server store legs of tests/test_uploads_shard_quota_interleave.py were not run locally and must be read on CI.
…unded; the straggler drain reads its defaults when it runs

BACKLOG #3740 items 4, 11 (the uploads.py text) and 12 (the drain_stragglers part).

Item 4. UploadStore.save's cancel arm removed a landed pair under
_quota_lock with no bound on the wait, so one hung unlink queued every
upload in the process. _remove_cancelled_pair now waits at most
_LEDGER_CANCEL_WAIT_SECONDS on the executor's own Future, then lets the
lock go with the removal still running; until it lands the pair is still
billed, which errs toward refusing. A late failure is logged at ERROR by
file_id. _reserve_across_shards's awaited release (item 5) is untouched.
The thread submission is shared with _to_thread_to_completion through
_submit_to_thread. Test: a hung removal does not block a second upload's
reserve; red on origin/main.

Item 12. drain_stragglers reads _STRAGGLER_DRAIN_SECONDS and
_STRAGGLER_CANCEL_GRACE_SECONDS at call time. Test red on origin/main.

Item 11, uploads.py: the UploadQuotaError residual and the
_reserve_may_hold_a_slot log lines say a release is started, not that
the slot is paid back; one rewritten docstring line qualifies "engine
shard".
… both lifespans' drains, the schema DDL's drop, and engine shard qualified

BACKLOG #3740 items 3 (reason comment), 11 (the remaining text) and 13.

Item 3: upload_file's outliving.run(_record_upload_create...) site says
why a deadline that answers 503 keeps the stored file and its row, so a
retry stores a second copy (kept by design, the owner's delegated ruling
of 2026-10-29). Comment only.

Item 11: the outliving = OutlivingOperations() comment names the upload
routes and the embedded lifespan; CHANGELOG.md's #3180 entry says the
schema DDL drops upload_quota, at open on SQLite and under auto, and by
provision-schema under external; "shard" qualified in the lines PR 2362
rewrote (store comments, the interleave test).

Item 13: api/outlive.py's module, drain and DRAIN_TIMEOUT_SECONDS text
covered only DR and reload arms; api/approvals.py's Executor text named
only the replay's row as committed with its effect, not the purge's.
…bits path, and states its cost once

The caveat no longer prints the raw zone, which ipaddress takes almost
unchecked; the entry is already quoted escaped. The host-bits refusal of
a zoned entry (fe80::1%eth0/64) adds it too. The cost moves to one place,
the trusted_proxies row of CONFIGURATION.md, which drops "fails closed":
the inner proxy's address is what the audit, the login limiter and
allowed_client_networks then see. It also records that fe80::1/128 matches
a zoned hop on any zone, pinned by a test. ADR 0151's amendment no longer
calls ::/80 and ::/0 outside the mapped block.

BACKLOG #3764 items 1, 4, 5 (confirmed met), 7, 8, 9, 10 (reported), 12, 13, 14 and 15.
Proposed PR title: Trusted-proxy hygiene: the zoned-hop gap is stated beside its refusal, ADR 0151 D-1 says what uvicorn is handed
Proposed banner: Items 1, 4, 7, 8, 9, 12, 13, 14 and 15 built; item 5 confirmed already met; item 10 reported (alert 209 would need re-dismissal after merge). Open: item 6 (a uvloop IPV6_V6ONLY measurement on a Linux host), item 11 (the CodeQL alert 209 dismissal, a repository act), and folding a zone off an XFF hop, a follow-up. Items 2 and 3 ride PRs 2638 and 2634.
…unts

Review round 1 on the release change. release_upload_quota takes
uploader_id and reservation_id keyword-only, on the Store protocol,
the UploadQuotaLedger protocol and all three backends, since its only
caller holds the two ids in the opposite order and a swap of two strs
would pass mypy. The offline SQL Server and PostgreSQL fakes now
assert one bound value per placeholder. The SQL Server release
docstring and the changelog state the no-row cost: four statements
rather than two, and a possible wait on a sibling's reserve.

BACKLOG #3740 items 10 and 12 (item 12: the SQL part only).

Proposed PR title: batch 460 wave 2 (BACKLOG #3740, #3764)
Proposed banner: Item 10 built: the upload-quota release takes the uploader (keyword-only) on all three backends, deletes only that uploader's row, and on SQL Server takes the uploader's applock with no read to learn it; a release with a row sends as many statements as before, and one with no row sends four rather than two. Item 12's SQL part built: the reserve's COUNT and SUM are one aggregate on every backend. The PostgreSQL and SQL Server legs of tests/test_uploads_shard_quota_interleave.py were not run locally and must be read on CI.
…failure

Review round 2. The SQL Server release docstring no longer claims the
release reads nothing (the opener reads the uploader's index, unlocked)
or that a caller wait bounds the no-row case (command_timeout = 0
removes the applock timeout). The changelog counts round trips, and
says the PostgreSQL and SQLite releases take no lock. The budget-edge
contract test releases its rows in a finally, so a failing server leg
leaves nothing in flight.

BACKLOG #3740 items 10 and 12 (item 12: the SQL part only).

Proposed PR title: batch 460 wave 2 (BACKLOG #3740, #3764)
Proposed banner: Item 10 built: the upload-quota release takes the uploader (keyword-only) on all three backends and deletes only that uploader's row; on SQL Server it takes the uploader's applock with no read to learn it, so a release with a row makes as many round trips as before and one with no row makes four rather than two. Item 12's SQL part built: the reserve's COUNT and SUM are one aggregate on every backend. Open: the PostgreSQL release still takes no per-uploader lock, and no test races a release against an uncommitted reserve. The PostgreSQL and SQL Server legs of tests/test_uploads_shard_quota_interleave.py were not run locally and must be read on CI.
…/128, and point at one source

CONFIGURATION.md said clients behind the gap are seen as the inner proxy;
they are seen as the zoned hop's address, which the row now says. The row
and the caveat no longer offer fe80::1/128 as the entry that matches: that
is an open question, since the zoned-network refusal names trusting an
address on every interface as its reason. A test still pins the /128
behaviour as a measurement. _ZONED_HOST says "no socket peer". ADR 0151's
amendment and index note point at uvicorn_trusted_hosts instead of
restating its twin rules.

BACKLOG #3764 items 1, 4, 5 (confirmed met), 7, 8, 9, 10 (reported), 12, 13, 14 and 15.
Proposed PR title: Trusted-proxy hygiene: the zoned-hop gap is stated beside its refusal, ADR 0151 D-1 says what uvicorn is handed
Proposed banner: Items 1, 4, 7, 8, 9, 12, 13, 14 and 15 built; item 5 confirmed already met; item 10 reported (alert 209 would need re-dismissal after merge). Open: item 6 (a uvloop IPV6_V6ONLY measurement on a Linux host), item 11 (the CodeQL alert 209 dismissal, a repository act), whether to advise fe80::1/128 for a zoned hop, and folding a zone off an XFF hop, a follow-up. Items 2 and 3 ride PRs 2638 and 2634.
…e 0.5.2 note correction moves to a fragment, and text rewrapped

BACKLOG #3740 items 4, 11 and 13, code-review round 1 repairs.

- _remove_cancelled_pair waits _CANCELLED_REMOVAL_WAIT_SECONDS, a
  filesystem bound apart from the ledger's, so tests and tuning of
  _LEDGER_CANCEL_WAIT_SECONDS no longer move it.
- Past the bound an ERROR names the file_id as a stored file with no
  upload.create row, since the process may stop before the removal
  answers; a late removal logs INFO, a failed one a second ERROR.
- The test asserts that ERROR line, not a substring a ledger WARNING
  shares, and waits for the stragglers before the in-flight check.
- The 0.5.2 "dropped at open" correction is a changelog.d fragment;
  CHANGELOG.md is back as released (changelog.d/README.md).
- api/outlive.py lists the arms with "at least".
- Paragraphs the earlier splices left long are rewrapped.
…e residuals and the outliving arms are stated plainly

BACKLOG #3740 items 3 (reason comment), 4, 11, 12 (drain_stragglers
defaults) and 13. Code-review round 2 repairs:
- the late removal reads back, in its thread, whether the pair is gone,
  and a WARNING withdraws the ERROR, at a level the default log keeps;
- _remove_cancelled_pair names its residuals: an abandoned removal holds
  an executor worker, uncapped, and a scan can refuse its Windows unlink;
- save()'s docstring covers the abandoned removal;
- api/outlive.py says the upload.create arm starts a removal that can be
  cut short; drain() and the outliving comment point, not restate;
- the fragment says "no other upload in that process".

Proposed PR title: Batch 460 wave 2: a cancelled upload's file removal no longer holds the quota lock unbounded, plus upload and outliving text fixes (BACKLOG #3740 items 3, 4, 11 to 13)

Proposed banner: PARTIAL. Built: item 4 (the cancel-path removal waits at most _CANCELLED_REMOVAL_WAIT_SECONDS under _quota_lock, then goes on without it, with an ERROR by file_id; test red on the unfixed code); item 3 kept by design with the reason in the code at upload_file's outliving.run site, per the owner's delegated ruling of 2026-10-29; item 11 (UploadQuotaError residual, the reserve log lines, the outliving comment, a changelog.d correction of 0.5.2's "dropped at open", "engine shard" in the lines PR 2362 rewrote); item 12's drain_stragglers defaults (test red on the unfixed code); item 13 (api/outlive.py and the approvals.py Executor text). Item 8 was already fixed by the server-clock SQL (#3256). Not built here: item 12's discard reuse, since discard is what PR 2637 rewrites; item 12's COUNT/SUM and teardown parts (other wave 2 Builders); items 1, 2, 5 to 7, 9, 10, 14 to 18 per their own rows and PRs. Open: discard()'s removal after a failed upload.create row still waits unbounded under the lock, so a hung unlink there would still queue every upload; an abandoned removal would hold an uncapped default-executor worker; docs/PHI.md's uploads row does not yet list a removal abandoned at the bound.
…beat touches, and the step list is pinned

The upload straggler drain now waits, inside its own wait share, for the
outliving operations still running (OutlivingOperations.settle), so a
removal a cancelled upload.create row's arm starts late is held and
drained before the step returns. The drain no longer waits for a
heartbeat touch: each pass cancels held touches, and a cancelled
heartbeat cancels its own. The managed teardown's steps are built by
_managed_teardown_steps, so a test pins their order against the real
list, and the NSSM-window test counts a cut grace per bounded step.
drain_stragglers_within's split is derived from its defaults; the
long-operations split is the named _OUTLIVING_WAIT_SHARE.

BACKLOG #3740 items 6, 14, 15, 16, 17, 18, and item 12's teardown part
(recorded gone). Item 15 is also BACKLOG #3819 item 1.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within (derived) and named, not derived, for the long-operations split, by design; item 12's teardown part was already gone. Item 15 would leave 0.0 s of headroom on first deployment: 10 bounded steps make 10.0 + 3.0 + 1.0 = 14.0 s, plus the 1.0 s exit room, exactly the 15.0 s NSSM window; no constant moved.
…lls in its warning

Review of the wave-2 rebase found the cancel pass over what the first
wait left also cancelled a touch _cancel_touches had already cancelled,
which would cut short a store's unwind of the first cancel. The pass
now skips touches. The warning's count leaves touches out, and a
WARNING names touches still unwinding when the drain returns.

BACKLOG #3740 item 14.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within, and its long-operations split is named, not derived, which stays open for a ruling; item 12's teardown part was already gone. Item 15 would leave 0.0 s of NSSM headroom on first deployment: 14.0 s plus 1.0 s exit room equals the 15.0 s window.
…led heartbeat cancels its touch once

Review round two of the wave-2 rebase: the touches stayed uncancelled
for the whole settle wait, and a heartbeat's finally cancelled a touch
that something else may already have cancelled. Both now go through
_cancel_touches, which cancels each touch once.

BACKLOG #3740 item 14.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within, and its long-operations split is named, not derived, which stays open for a ruling; item 12's teardown part was already gone. Item 15 would leave 0.0 s of NSSM headroom on first deployment: 14.0 s plus 1.0 s exit room equals the 15.0 s window.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, W2a (BACKLOG #3740 items 3, 4, 11, 13, 12 part; head f7cd683977), posted verbatim by the batch 460 Manager:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a4101e605a4513d08 round-c68a1be723, a6f055f0725231c06 round-7e9397eb49; ascii spelling).
Rounds: 2. Findings: round 1, 13: 8 fixed (ERROR at the bound, separate bound constant, test log assertion, test straggler wait, CHANGELOG.md edit moved to a fragment, "at least" in outlive.py, line rewraps; discard path stated in the docstring and fragment), 5 not done (discard path, executor exhaustion, next save hangs on the same filesystem, OutlivingOperations.drain def-time defaults, UploadQuotaError shard sweep). Round 2, 12: 6 fixed (late-success WARNING that reads the result back, residuals named in the docstring, save() docstring, outlive.py "starts the removal", restatement trimmed, fragment says "in that process"), 6 open or rejected (below).

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, W2b (BACKLOG #3740 items 10, 12 SQL; head 2efc4deb75), posted verbatim by the batch 460 Manager:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a49bf3cbf05ebc642 round-27f6afa639, aabedfb5870745448 round-c3c9165e1a).
Rounds: 2. Findings: 6 confirmed and fixed (keyword-only release ids; placeholder-count checks in the fakes; changelog round-trip accuracy, twice; docstring bound and no-read wording; budget-edge test cleanup in a finally), 6 rejected (no-row release now waits on the applock, raised in both rounds: kept, because it is what waits out a reserve that has the lock but has not inserted yet, and documented; a wrong uploader is a silent no-op: idempotent by contract, the keyword-only change guards the swap; index-hinted delete instead of applock: unmeasurable without a server; six copies of the aggregate, and the opener helper near-copying the audit one: mirrors the existing pattern; the edge test opens a two-store pair: consistent with its sibling), 6 open (below).

The Builder's tag line used Unicode arrows; this copy writes them as -> and <=, with nothing else changed.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, W2c (BACKLOG #3740 items 6, 12 teardown, 14 to 18; head 98cdc0e5e7), posted by the batch 460 Manager with ONE correction. The Builder's round-2 fork id was a placeholder (a6e3c1b0). The Manager read it with korus scripts/quality/review_tag.py --target round-052c9490c7 --worktree <W2c worktree>: exit 0, fork a3c7ba8f3c4543861, tag xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings.

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a826351aa7f8bb66f round-b446769377, a3c7ba8f3c4543861 round-052c9490c7; ascii spelling).
Rounds: 2. Findings: 5 confirmed and fixed (double cancel in the cancel pass, warning count including touches, touches left running through settle, unguarded heartbeat cancel, a missing positive control in round one's own test set), 0 rejected, 9 open.

Both rounds cover the rebase resolution. The last commit, 98cdc0e5e7, came after round 2. The original branch's rounds 1 and 2, over a404092ad4, were forks a630a8f76cb72ba70 round-329742b3eb and aee8ff7cfe56c01ca round-9fb6f9dd7c.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, W2d (BACKLOG #3764; head eec940cdb3), posted verbatim by the batch 460 Manager:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks ab071c2b13847ee75 round-26310ad254, a6d7bd3f0cf053ee1 round-d929bde46c; ascii spelling).
Rounds: 2. Findings: 14 confirmed and fixed (raw zone echo, host-bits path lacked the caveat, "fails closed" false under allowed_client_networks, wrong host named, /128 doc recommendation withdrawn, ::/80 "outside" wording, restated twin rules in ADR + index, _ZONED_HOST "no peer", test docstring overclaim, dual isinstance, etc.), 3 rejected (moving uvicorn_trusted_hosts is unsafe: load validates every entry and settings.api is never replaced; reading messages after redaction weakens the negative assertion: item 15 mandates that read, and positive assertions guard it; CodeQL 209 unaddressed: items 10/11 are by brief not mine), 5 open.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander: handing over PR 2645 (MEFORORG/MessageFoundry), the batch 460 wave-2 batch PR. It carries four items, and none of them changes a security control.

  • Head SHA: c204103a41b1b89f0ed119d0b6236eea2c9cfed3. Each item's source head is in the body.
  • Unread legs:
    • W2b: the PostgreSQL and SQL Server store legs.
    • W2c: windows-service-smoke and the Windows pytest legs (timing-based tests).
    • W2d: CodeQL alert 209, which likely needs re-dismissal after merge (#3764 item 11).
    • The full suite was not run. The combined tree passed 1092 targeted tests.
  • Review coverage: W2c's last commit 98cdc0e5e7 is not covered by a review round. W2c's resolution of its conflict with W2a was reviewed in its own two rounds.
  • Known defects: listed per item in the body.
  • Landing order: none. Expect textual overlaps with 2637, 2640 and 2641.
  • Ledger: see Ledger asks in the body. There are two new rows, and one note goes on #3819.

@wshallwshall wshallwshall added qa Builder QA record posted; not a merge gate and removed qa Builder QA record posted; not a merge gate labels Oct 29, 2026
One conflict, in messagefoundry/api/app.py's imports: main's new
refused_body import line is kept, followed by the batch's multi-line
security_notify import, which adds SecurityEventNotifier.

BACKLOG #3740 (batch 460 wave 2).
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session d1c562bf), 2026-10-29. Held for three fixes and a conflict with main, at c204103a41b1b89f0ed119d0b6236eea2c9cfed3. The Lander's review (round-535741f7b5, xhigh, review_tag exit 0) covers the whole head, W2c's 98cdc0e included. SQL is parameterized on all three stores. The cancel path re-raises after cleanup. The trusted-proxies text matches the code: a zoned hop stays untrusted.

Fix before merge (each changes behaviour against main)

  1. uploads.py:1607 and api/app.py:1387. settle=outliving.settle waits on every running long operation (DR, reload, delete), not only upload.create rows, and gets the straggler drain's whole wait. If a DR or reload arm is running at shutdown, ledger reserves and save-time sweeps are cancelled at about 0 s, where main gave them the full wait. A cancelled reserve then holds its slot for 300 s, and a cancelled sweep loses its upload.prune rows to a log line. Scope settle to upload rows, or give the drain its own budget.
  2. uploads.py:1550. A removal abandoned at the 5 s bound keeps a default-executor worker, with no cap. On a filesystem whose unlinks hang, repeated cancels can exhaust the pool every asyncio.to_thread in the engine shares. That turns an uploads stall into an engine stall. Cap the abandoned removals, or run them on a dedicated bounded executor.
  3. uploads.py:1553. contextlib.suppress(CancelledError) inside a while under _quota_lock busy-spins for up to 5 s if the deadline re-delivers the cancel on each pass, as anyio does, while every other upload queues on the lock. Absorb the cancel once, or sleep or await between passes.
  4. The conflict. git merge-tree against main conflicts in messagefoundry/api/app.py. Merge origin/main.

Rows (file after merge)

  • uploads.py:1565: an abandoned removal is invisible to the drain, and its withdrawing WARNING can be lost.
  • __main__.py:4924: the late uvicorn_trusted_hosts call. No unvalidated path was found.
  • uploads.py:1596: ZeroDivisionError only with both defaults at 0.
  • Design and duplication cleanups at app.py:1387 and uploads.py:1651, 1552 and 718.
  • The body's other open findings.
    The settings.py '/128' suggestion is rejected under decision d20261029T223607035-82fdc4.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line for the merge of main into the wave-2 batch (Builder W2c, head 5adfe893b1, covering c204103a41..HEAD). Posted verbatim by the batch 460 Manager:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (fork ac4a0badb05118176 round-202591efa3; ascii spelling).
Rounds: 1. Findings: 0 against the resolution; 8 shipped open, all where main's changes meet the batch's or in code main brought in.

The fork transcripts are under C:\Users\Scott\.claude-account-7\projects (--projects-dir).

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander: the api/app.py conflict on PR 2645 is resolved. The new head is 5adfe893b15019f73126bf8a94498991706074f5, a fast-forward from c204103a41 (not forced). It is a merge of origin/main at 1110d592d3.

  • One hunk, the imports at about line 460, resolved as a keep-both. It keeps main's from messagefoundry.pipeline.refused_body import RefusedBodyPolicy, head_only_resubmit_refusal, then the batch's multi-line security_notify import, which adds SecurityEventNotifier. Ruff's import order is clean. No other file conflicted. The reviewer re-ran git merge-tree: the committed tree differs from the automatic merge only by that hunk.
  • Checks. Ruff is clean, and mypy is clean on the linux and win32 legs. pytest ran 315 passed and 10 skipped, over the lifespan-teardown tests, every test_upload*.py, and main's refused_body tests. The full suite was not run.
  • Hosted legs to read: windows-service-smoke, the Windows pytest legs, the PostgreSQL and SQL Server store legs, and the timing-quarantine job (item 6 below).

Open findings where main meets the batch. All are for rows. None is from the merge itself.

  1. UploadStore.discard still waits on its unlink under _quota_lock with no bound. After PR 2637, every failed upload.create row goes through it, and W2a bounded only the save path. This is the same as the new row already asked for in the body.
  2. In _unlink_then_row (PR 2637): a delete whose sidecar unlink raises some other OSError would answer 500 with no upload.delete row.
  3. No teardown step waits for the upload.delete row that _discard_and_record writes. On a late removal, it would meet a closed store and be logged at ERROR.
  4. Stale premise: outlive.py's module docstring, and the upload-step comment in app.py at about lines 1429 to 1431, both say a removed file has no creation row. After PR 2637 it can have one.
  5. _discard_under_lock duplicates W2a's _submit_to_thread, and its comment is stale.
  6. Main's tests/timing_quarantine.txt takes test_a_timed_out_stop_keeps_the_row_of_every_pair_already_removed off the gating legs. Read its timing-quarantine result.
  7. Two stubs of _discard_cancelled_sync in tests/test_upload_reservation_identity.py return None, but the function now returns a bool.
  8. Efficiency: the approval rejoin re-parses params up to three times, and a gated purge scans twice.

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 30, 2026
@wshallwshall wshallwshall removed the qa Builder QA record posted; not a merge gate label Oct 30, 2026
…ve waits for them instead of the cancelled one spinning

BACKLOG #3740 item 4, the Lander's hold on the wave-2 batch PR (2645),
findings 2 and 3.

2. A removal abandoned at the bound kept a default-executor worker with
no cap. Removals now run on _removal_executor, a pool of
_CANCELLED_REMOVAL_WORKERS (2); a hung one past the cap queues there and
still runs, and the pool every asyncio.to_thread shares stays free.
Test: three hung removals with a one-worker pool run one at a time; red
on 5adfe89 (three ran at once on the default executor).

3. The wait absorbed every CancelledError in a loop under _quota_lock, so
an anyio scope, which cancels on every loop pass, spun it for the whole
bound. The cancelled save now stops waiting at the next cancellation.
The removal is registered in _pending_removals with its deadline, and the
next save waits for it, holding the lock and not cancelled, before it
reserves and scans, so the pair is still not counted. The ERROR at the
bound is a timer. Test: under an anyio cancel with a hung removal, A ends
well inside the bound and B waits; red on 5adfe89 (A ran out the
bound). test_an_anyio_scope_cancel_still_removes_the_landed_file now
waits for the removal, which can end just after the save does.
…unning, waits are per uploader, and a failed start is logged

BACKLOG #3740 item 4, code-review round 1 repairs over 5adfe89..HEAD.

- The bound's ERROR is a loop timer a closing loop would drop, so
  drain_stragglers names every removal still inside its bound at ERROR
  first; a removal that lands later logs the withdrawing WARNING. Test.
- A pending removal leaves _pending_removals at its bound or at the
  drain, not only when it ends, so a removal that never returns holds no
  later save and no entry for the life of the store.
- A save waits only for its own uploader's removals; the scan counts that
  uploader's files only.
- save's arm logs a removal that could not start (submit can raise) and
  still propagates the cancellation.
- Docstrings: the cap's cost (removals queue behind hung ones, bound
  counted from queueing), the suppress's one caller, the WARNING's text.
- Tests: the cap test shuts its pool in its finally and drops a check
  that passed unfixed too; the spin test's bound is 5 s; the two other
  anyio tests in test_uploads.py wait for the removal.
… names only what its wait leaves, and the withdrawal is logged from the thread

BACKLOG #3740 item 4, code-review round 2 repairs over 5adfe89..HEAD.

- Every save waits for pending removals again, not only the same
  uploader's: the scan opens every sidecar, which on Windows refuses the
  unlink. The deadline filter is back, so an entry whose timer never ran
  cannot hold a save.
- The drain waits for pending removals inside its own wait and names at
  ERROR only those still running at its end, so a healthy unlink is not
  reported as an audit gap.
- The withdrawing WARNING is logged in the removal's thread, so it still
  lands after the loop has closed.
- A removal that cannot start is caught as RuntimeError and its ERROR
  says the file is stored with no upload.create row. Test.
- The cap test also checks the real pool's cap. The changelog fragment
  and the docstrings describe the next-save wait.
…very long operation

The Lander's hold on the wave-2 batch: the settle before the upload
straggler drain waited on every running long operation, so a DR change
or reload still rolling back at shutdown held the step for its whole
wait share, slack the later audit-writing steps would otherwise get.
OutlivingOperations.settle takes a label, and the step passes the
upload route's own, now the constant _UPLOAD_CREATE_OPERATION.

BACKLOG #3740 item 6.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within, and its long-operations split is named, not derived, which stays open for a ruling; item 12's teardown part was already gone. Item 15 would leave 0.0 s of NSSM headroom on first deployment: 14.0 s plus 1.0 s exit room equals the 15.0 s window.
…arrowing costs

Review of the settle narrowing: a default that waits on every operation
made the retired behaviour the easy path, and the comment named only the
slack it saves, not that a DR arm in its hook reap loses this wait.
Docstring and changelog lines rewrapped at 100 columns.

BACKLOG #3740 item 6.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within, and its long-operations split is named, not derived, which stays open for a ruling; item 12's teardown part was already gone. Item 15 would leave 0.0 s of NSSM headroom on first deployment: 14.0 s plus 1.0 s exit room equals the 15.0 s window.
… revision

Review round two of the settle narrowing: the step comment, the new
test's docstring and the changelog compared the narrowed settle with
the wait-on-everything form, which never reached main. They now state
the design, and the test docstring says "at least" where the constant
does. Prose only.

BACKLOG #3740 item 6.

Proposed PR title: Shutdown drains: the upload drain settles long operations and skips heartbeat touches; the teardown order and NSSM window are pinned against the real steps (BACKLOG #3740 items 6, 14-18; #3819 item 1)

Proposed banner: PARTIAL. Items 6, 14, 15, 16 and 18 are built; item 17 is built for drain_stragglers_within, and its long-operations split is named, not derived, which stays open for a ruling; item 12's teardown part was already gone. Item 15 would leave 0.0 s of NSSM headroom on first deployment: 14.0 s plus 1.0 s exit room equals the 15.0 s window.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA lines for the PR 2645 hold fixes. Posted verbatim by the batch 460 Manager.

W2a, fixes 2 and 3 (5adfe893b1..d35b35b747):

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks ab329d1674a7814aa round-c95774e239, a90373ae3f1a40eff round-c48fd12ad8; ascii spelling).
Rounds: 2. Findings: round 1, 14: 9 fixed (drain names pending removals, entries leave at the bound, per-uploader wait (reverted in round 2), failed start logged and cancel propagates, other anyio tests wait for the removal, cap cost documented, suppress scope documented, WARNING text, test pool shutdown in finally, invalid control dropped, spin-test bound widened), 4 rejected or out of scope (the cancelled save's own wait kept: the next save waits for the same removal to the same deadline, so it adds no lock time and keeps plain-cancel removals finished on return; discard() path; _discard_under_lock reuse; cache freezing the cap, now documented). Round 2, 14: 9 fixed (every save waits again, Windows sidecar; deadline filter restored; drain names only what its wait leaves; withdrawal logged from the thread; RuntimeError narrowed; failed-start ERROR names the audit gap; stale docstrings and fragment; failed-start test; real pool cap checked), 5 open (below).

W2c, fix 1 (d35b35b747..facaafad5b):

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks ae890a56d31aefd88 round-48592eb4be, a3e1e1088b1cf18f3 round-61dbff3b56; ascii spelling).
Rounds: 2. Findings: 7 confirmed and fixed (settle default made required, trade-off stated in the step comment, docstring ambiguity, 100-column wraps, test bound widened against Windows timer jitter, prose compared with an unshipped revision, a completeness claim softened to "at least"), 0 rejected, 4 open.

No review round covers facaafad5b, which changes prose only. Fork transcripts are under C:\Users\Scott\.claude-account-7\projects (--projects-dir).

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander: all three hold fixes have landed. The new head is facaafad5b53fe3f17f37194c73b53221dde4b3b, reached by fast-forwards from 5adfe893b1 and never forced. Fix 4 (the conflict) was answered earlier.

Fix 1: settle now waits only on upload rows (W2c: 54ef10a15f, c3a40185e6, then the prose-only facaafad5b).

  • OutlivingOperations.settle(timeout, *, label) requires a label and waits only for operations with that label. The upload straggler step passes label=_UPLOAD_CREATE_OPERATION, which is "upload.create audit". A DR change or reload that is still rolling back gets only the long-operations drain's grace, and no longer holds the upload step.
  • The Builder corrected the hold's premise. Reserves and sweeps were not cut to about 0 s of wall time: they kept running through the settle and were cancelled at about main's moment. The real defect: a stuck DR or reload arm held the upload step for its whole wait share after every straggler had finished, taking slack from the later steps that write audit rows.
  • Test: tests/test_lifespan_teardown.py::test_a_stuck_non_upload_operation_does_not_hold_the_upload_straggler_drain. On d35b35b747 it is red: "the upload drain waited on the DR arm: 1.47s". At head it is green in about 0.35 s, with a 1.1 s bound.

Fix 2: abandoned removals are capped (W2a, on the way to d35b35b747).

  • Cancelled-pair removals now run on a dedicated pool, _removal_executor(), with _CANCELLED_REMOVAL_WORKERS = 2. Hung unlinks can hold at most two threads, and the default executor stays free.
  • Test: test_abandoned_removals_run_on_their_own_capped_pool. On 5adfe893b1 it fails with "3 hung removals ran at once".

Fix 3: no busy-spin (W2a). The cancelled save absorbs the cancel once, so under anyio it returns at once. The removal is registered in _pending_removals with its deadline. The next save waits for it under _quota_lock, without absorbing cancels. At the bound, a loop timer logs an ERROR naming the file_id. drain_stragglers names any removal still running at the end of its wait.

  • Tests, each failing on 5adfe893b1:
    • test_a_repeated_cancel_does_not_spin_a_cancelled_save_on_its_removal ("A waited out the bound under repeated cancels")
    • test_the_drain_names_a_removal_still_running
    • test_a_removal_that_cannot_start_is_named_and_the_cancel_still_propagates
  • Behaviour change: while a removal is pending, every save in the process waits up to 5 s under _quota_lock, so that it never counts a pair about to go. Before this change, the cancelled save itself held the lock for as long as the unlink took. Three existing anyio tests now wait for the removal through _removals_settled.

Checks. Ruff clean. mypy clean on the linux, win32 and tests legs. 72 changelog fragments, all well-formed. pytest: W2a's upload and teardown set, 354 passed; W2c's teardown, outlive, reload, DR, partition and upload set, 326 passed.

Hosted legs: windows-service-smoke and the Windows pytest legs (timing-based tests), and the PostgreSQL and SQL Server store legs.

New open findings, for rows:

  • The settle chooses its operations by log label, so a path that runs _record_upload_create under another label would silently drop out of the wait.
  • The settle scope is restated in several places (SDS-3.5).
  • No test covers a stuck DR arm and an unwinding upload.create together.
  • tests/test_upload_api.py uses the literal "upload.create audit" twice.
  • The lock-wait cost described above.
  • A repeat-cancel absorption, for its one caller.
  • Queued removals count their bound from enqueue.
  • _removals_settled treats an entry dropped at its bound as settled.
  • discard()'s unbounded removal is the new row already asked for.

@wshallwshall wshallwshall added qa Builder QA record posted; not a merge gate and removed qa Builder QA record posted; not a merge gate labels Oct 30, 2026
One conflict, the `trusted_proxies` row of docs/CONFIGURATION.md. The
resolved row is main's row, with its exposure-rule sentence (vault
BACKLOG #2251), plus the batch's two insertions: "socket" in "would
match no socket peer", and the zoned-hop caveat (BACKLOG #3764). A
word diff of the row against each side shows only the other side's
insertions.

BACKLOG #3740 (batch 460 wave 2).
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session d1c562bf), 2026-10-30. Fixes 1-3 covered at facaafad5b53fe3f17f37194c73b53221dde4b3b. An independent review found all three MET and CLEAN.

  • Fix 1. settle filters to named == label (outlive.py:162). The upload step passes _UPLOAD_CREATE_OPERATION (app.py:1448), and the later steps keep their budgets.
  • Fix 2. Removals run on a ThreadPoolExecutor(max_workers=2) (uploads.py:111-120). Work past the cap queues, and the pool is deliberately not shut down, which is documented.
  • Fix 3. One suppress around one asyncio.wait (uploads.py:1634), with no loop. The follow-on wait's timeout only shrinks.
  • Tests. All five new tests fail on the pre-fix code and pass at the head. The suites pass: 117 tests across lifespan teardown, upload reservation and uploads.
  • Rows. The 'upload delete' operation is not settled, the same as main (open finding 3). A hung unlink blocks process exit through the pool join, the same as on main.

Still needed: the conflict with main. It is now in docs/CONFIGURATION.md, after 2638, 2641 and 2644 merged. Merge origin/main and reply. The Lander checks the resolution and then enqueues.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line for the second sync of main into the wave-2 batch (Builder W2c, head 06dc38df01, covering facaafad5b..HEAD), posted verbatim by the batch 460 Manager:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (fork a975c6533c226a6a3 round-f1f67c79cb; ascii spelling).
Rounds: 1. Findings: 0 against the resolution; 7 shipped open, all in main's own changes this merge brings in.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander: the second conflict on PR 2645 is resolved. The new head is 06dc38df01b3e739df4a7e3e5b650884d0975a6b, a fast-forward from facaafad5b that merges origin/main at dbfb3cad3b.

The conflict. Only docs/CONFIGURATION.md conflicted, in the single trusted_proxies row. Against the merge base 1110d592d3, each side only added text:

  • The batch (W2d) added "socket" in "would match no socket peer", and the zoned-hop caveat.
  • Main (PR 2641) added the exposure-rule sentence, "A non-empty list also counts the instance as exposed, on a loopback bind too ...".

The resolution. The resolved row is the batch's row with main's sentence where main placed it. A word diff of the result shows only the batch's two additions against main, and only main's sentence against the batch. The reviewer re-ran that comparison and found nothing dropped. The automatic merges in __main__.py, api/app.py, api/approvals.py and config/settings.py are clean.

Checks. ruff passed. pytest passed 1129, with 1 skipped, across these files:

  • the CONFIGURATION.md doc tests (test_exposure_rule_doc_drift, test_security_doc_drift);
  • test_api_tls, test_security_cli, test_security_posture_defaults, test_settings;
  • test_client_network_allowlist, test_off_loopback_runbook, test_lifespan_teardown.

Open findings in code this merge brings in from main. These are for rows:

  1. #3859, a behaviour item. A refused operator start of an auto_start=false listener leaves a failed record. The engine then stays DEGRADED even after a reload fixes the cause, because the reload's NOT_STARTED branch skips the _failed.pop. Round 2 of PR 2642 kept main's reload branch on purpose: popping it there hid a down feed. So this is the trade-off in the other direction, and it needs a ruling, not a quick fix.
  2. restart_inbound's docstring says an operator restart records nothing. That is now false.
  3. restart_inbound's own _record_failed block is now unreachable.
  4. _checked_inbound_start records a failure and alerts even on a stopped runner, where restart_inbound deliberately did not.
  5. In the docs/CONFIGURATION.md row, main's sentence now separates "That re-encrypting shape owes..." from the sentence it refers to.
  6. The __main__.py dual-control advisory's enabled-off text hardcodes the default pair of operations.
  7. ws_allowed_origins loads https://xn---style hosts.

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 30, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session d1c562bf), 2026-10-30. Resync checked at 06dc38df01b3e739df4a7e3e5b650884d0975a6b. The merge's only remerge-diff hunk is the docs/CONFIGURATION.md trusted_proxies row. It keeps the branch's zoned-hop text (#3764) and main's 'A non-empty list also counts the instance as exposed' sentence (vault #2251), and drops nothing. git merge-tree against current main exits 0. Every hold fix is covered at facaafa. It enqueues once the nine required contexts are green on this head.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session d1c562bf), 2026-10-30. Enqueuing at 06dc38df01b3e739df4a7e3e5b650884d0975a6b. All nine required contexts are green on this head: the checked keep-both resync onto the covered facaafad5b53fe3f17f37194c73b53221dde4b3b.

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit ef34a6e Oct 30, 2026
58 of 59 checks passed
@wshallwshall
wshallwshall deleted the batch/b460-wave2 branch October 30, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant