Skip to content

Batch 261 (api): the API lifespans' shutdown runs every step under one budget, and upload sweep, retention and quota follow-ups (BACKLOG #3191, #3184, #3240, #3256) - #2363

Merged
wshallwshall merged 10 commits into
mainfrom
batch/b261
Oct 17, 2026
Merged

wshallwshall merged 10 commits into
mainfrom
batch/b261

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Batch 261 (api), wave 1: the API lifespans' shutdown runs every step under one budget, plus upload sweep, retention and quota follow-ups (BACKLOG #3191, #3184, #3240, #3256).

Merge danger and evidence

  • Danger: the shutdown order changed. security_notifier.aclose() now runs BEFORE engine.stop(), not after. On main, its undelivered-notice audit rows went to a store engine.stop() had already closed. Every teardown step now runs under one budget of _SHUTDOWN_BUDGET_SECONDS = 10.0, with 3.0 s reserved for engine.stop(), so a step can be cut at its bound. The steps cut are the upload runner stop (about 2 s), the notifier closes, and the long-operations grace (about 2.9 s, below the 5 s DR hook reap, documented at outlive.py CANCEL_GRACE_SECONDS).
  • Danger: the NSSM stop window. 10 s budget + 3 s uvicorn drain (_UVICORN_DRAIN_SECONDS, new) + 1 s exit room = 14 s, inside AppStopMethodConsole 15000. engine.stop() is never cut, so its own internal waits (for example the 10 s graph-supervisor wait) can still overrun. windows-service-smoke exercises the real NSSM stop; read it.
  • Danger: SQL changes, server backends only. upload_quota.since is stamped and aged with the database server's clock on Postgres (statement_timestamp()) and SQL Server (SYSUTCDATETIME()). The SQL Server release reads the clock once into DECLARE @now, because two references in one statement can differ. The new server-clock test runs only on the Postgres and SQL Server legs.
  • Danger: a stacked PR follows. b261-reseal-on-batch (#3181) is stacked on this branch and lands AFTER it.
  • Evidence, combined tree (main 8ba57eeb + B + C2, before F's fixes): ruff clean; mypy clean on all four legs; full pytest -n 6 gave 47,612 passed and 12 failed. Those 12 were re-run serially on main 8ba57eeb and on this branch: 81 passed on each, so they are parallel-load flakes, not caused by this batch. After F's fixes: ruff and the four mypy legs are clean. F's full suite was spoiled by its own mid-run app.py edits (route-walking tests read source by line number). Every affected file re-run at the final head: 288 passed, 0 failed. The full suite has not run on the final head locally; CI runs it.

Items

BACKLOG #3191: approvals lifespan teardown (lane B)

Source branch b261-lifespan, head 0a0b923ca22ddb581507f53d2c10e0c0cf7b75cc.

  1. A cancel in mid-teardown no longer skips the later drains. Each step runs as its own task, and the cancel is raised at the end.
  2. engine.stop() and security_notifier.aclose() are guarded steps.
  3. Bounded, not guaranteed: see the NSSM danger above.
  4. The embedded create_app(engine=...) lifespan (now _embedded_lifespan) runs the same drain steps. It waits for a caller-owned auth service's lock notices but does not close the service.

Banner: PARTIAL. Items 1, 2 and 4 are built. Item 3: a 10 s budget that fits NSSM's 15 s with the uvicorn bound; engine.stop() is not cut and has no bound inside its 3 s reserve.

BACKLOG #3184: upload sweep and retention (lane C2)

Source branch b261-uploads, head dc580f1b0e1be68a0bf073cf2f29df415ef3cc12.

  1. A quota check walks the uploads directory once (_scan_metas_sync).
  2. Already done in d62308d8 (PR 2166).
  3. Each upload.prune row is written before the next pair is touched, so a timed-out stop loses at most the pair in progress.
  4. The upload route wakes the retention runner, and falls back to the inline sweep only when no runner is alive.
  5. Already done in d62308d8.

Banner: CLOSED. Items 1, 3 and 4 are fixed here; 2 and 5 were done in d62308d8 (PR 2166).

BACKLOG #3240: upload follow-ups from PR 2102 (lane C2)

  • Fixed: the sealed_plaintext under-count, the _UnsealableUpload docstring, the docs/PHI.md section 2 sentence, delete() answering 404 on a vanished sidecar, the refusal-hook test, the positive sidecar_refused store test, the delete() docstring, and the files:access_any plus files:delete plus step-up wording.
  • Already fixed before PR 2102 merged: the UploadQuotaError residuals note.
  • Declined: a transient Transit failure still answers 423. Telling it apart needs a CipherError subclass in store/crypto_transit.py.

Banner: CLOSED. The Transit-423 low was declined.

BACKLOG #3256: upload follow-ups from PR 2166 (lane C2)

  • Item 1: the server-clock since (see the SQL danger above).
  • Item 3: the heartbeat's touch wait is bounded (_RESERVATION_TOUCH_WAIT_SECONDS = 10), and a stop ends the wait.
  • The uvicorn drain: timeout_graceful_shutdown = 3.0.

Banner: CLOSED. Items 2 and 4 are held by #3180 and #3184.

Integration fixes (Builder F, on this branch)

Head c7e8f0912bbc454db77502a5643f94de2462697f, three commits on the merge of B and C2.

  • The retention runner's stop now honours its teardown bound.
  • A regression in the merged quota scan is fixed, with a test. A sidecar that is not UTF-8 only past the class check's read was never classed, so it stayed outside retention.
  • The SQL Server release reads the clock once.
  • The window test now reads _UVICORN_DRAIN_SECONDS directly.
  • Stale comments are corrected.
  • "cross-shard" is now "engine-shard" throughout uploads.py and the changelog, per CLAUDE.md section 12.

New items for the Lander to file

  1. auth/service.py close_background has no timeout, so a cut lock-notice drain can leave notices running. This is auth territory (batches 259/260).
  2. A cut notifier close can drop the MLLP and TCP deliveries refuse a payload holding more than one MSH (BACKLOG #2858, ADR 0209) #2383 undelivered-notice row for a send in flight. That is not worse than main, where those rows hit a closed store.
  3. engine.stop()'s internal waits can overrun the NSSM window.
  4. The long-operations cancel grace (about 2.9 s) is below the 5 s DR hook reap.
  5. A sidecar that is not UTF-8 only in its tail is pruned, but get_meta/delete raise, so DELETE /uploads/{id} returns 500.
  6. A FIFO or oversized .meta file can stall read_text under _quota_lock.
  7. After stop() cancels the retention sweep, its await task is unbounded.
  8. A transient Transit failure answers 423 rather than 503.
  9. "Opportunistically at save time" wording remains in docs/PHI.md, CONFIGURATION.md, CONNECTIONS.md and settings.py.
  10. _MSSQL_EPOCH_NOW and _PG_EPOCH_NOW duplicate the cluster lease's clock SQL.

Hosted legs to read

  • Postgres and SQL Server store legs (server-clock since, DECLARE @now, DATEDIFF_BIG, the derived-table MERGE).
  • windows-service-smoke (the real NSSM stop).
  • The connscale smoke leg.

…under one budget (BACKLOG #3191)

Both lifespans now tear down through _run_teardown in api/app.py. Each
step runs as its own task. A cancel of the lifespan reaches no step: the
running step finishes or is cut at its bound, the later steps run, and the
cancel is raised at the end. A step that raises is logged and the next
one runs. engine.stop() is never cut.

Every step shares _SHUTDOWN_BUDGET_SECONDS (10 s). It leaves NSSM's
15 s AppStopMethodConsole window room for uvicorn's drain, which #3256
bounds, and for interpreter exit. Each step keeps a floor.

The drains are one list, _app_drain_steps, and the embedded
create_app(engine=...) lifespan runs it too. The security notifier now
closes before engine.stop(), so its undelivered-notice rows reach an open
store.
…vice, never cut the task reap (BACKLOG #3191)

A step body that raises while it is built is now the step's own failure.
The embedded lifespan joins the lock notices of a caller's auth service
and does not close it. The store-reading tasks' reap is never cut.
outlive.py logs fractions of a second and names the budget.

Declined: KeyboardInterrupt and SystemExit leave the loop, as before.

BACKLOG #3191
PR title: The API lifespans' shutdown runs every step under one budget, and the embedded lifespan drains the same things (BACKLOG #3191)
Banner: PARTIAL. Items 1, 2 and 4 are built. Item 3: a 10 s budget
that fits NSSM's 15 s with #3256's uvicorn bound. engine.stop() is
not cut and has no bound inside its 3 s reserve.
…and SQL Server (BACKLOG #3256)

The reserve, the release and the zero-delta touch stamp and age
upload_quota.since with the server's clock (statement_timestamp() on
Postgres, SYSUTCDATETIME() on SQL Server) rather than the engine host's
time.time(), so engine hosts whose clocks disagree read one clock. SQLite
keeps the host clock, which is its server's. The gated server-backend test
is in the next commit, with the uploads layer that documents it.
…e runner, bounded touch wait and drain (BACKLOG #3184, #3240, #3256)

Proposed PR title: Upload sweep, retention, quota and drain follow-ups
from PRs 2102 and 2166 (BACKLOG #3184, #3240, #3256)

BACKLOG #3184
- 1 fixed: one directory walk per quota check (_scan_metas_sync `held`).
- 2 already done, d62308d (#2166): the late ERROR names each file_id.
- 3 fixed: each upload.prune row is written before the next pair; a
  timed-out stop loses at most the pair in progress. Tested.
- 4 fixed: the upload route wakes the running retention runner
  (UploadStore.wake_retention); with no runner it still sweeps inline.
- 5 already done, d62308d: _record_upload_prune is the one builder.

BACKLOG #3240
- sealed_plaintext counts a plaintext half sealed in a skipped pair.
- UploadQuotaError logging note: already fixed before merge (f6add0e).
- PHI.md item-2 sentence names the upload rotate-key does not seal.
- _UnsealableUpload docstring: "at least" in place of completeness.
- delete: a sidecar gone before the class check answers 404, not 423.
- route test pins one refusal-hook call (the drift alert); store test
  adds the positive sidecar_refused=True delete; delete docstring scope.
- PHI.md and 2322.fixed.md name files:delete plus step-up.
- declined: a temporary Transit decrypt failure still answers 423; telling
  it apart needs a CipherError subclass in store/crypto_transit.py, outside
  this lane, and 423 fails closed.

BACKLOG #3256
- 1 fixed in the previous commit; gated test here.
- 3 fixed: the heartbeat waits a bounded time for a touch, never cancels
  it, and never starts a second while one runs.
- uvicorn drain fixed: serve passes timeout_graceful_shutdown=3 s
  (_UVICORN_DRAIN_SECONDS), outside the lifespan.

Proposed banners:
#3184: CLOSED -- items 1, 3, 4 fixed in engine PR <n>; 2 and 5 were done
in d62308d (PR 2166).
#3240: CLOSED -- fixed in engine PR <n>; the Transit-423 low declined.
#3256: CLOSED -- items 1, 3 and the uvicorn drain fixed in engine PR <n>;
items 2 and 4 held by #3180 and #3184.
… prose (BACKLOG #3184, #3240, #3256)

- A touch's failure is logged from its own done callback, so one that
  lands after the wait is not left unretrieved; a stop ends the wait.
- delete() answers 404 only on FileNotFoundError (lstat), never for a
  sidecar that cannot be stat'ed.
- UploadQuotaError names the replica-clock residual and the never-returning
  touch; the orphan-sweep docstring no longer claims rows wait for it;
  wake_retention names the post-response quota window.
- _UVICORN_DRAIN_SECONDS states that any act-then-audit route can lose its
  row at the bound; PHI.md says "at least" for what rotate-key leaves.

Proposed PR title: Upload sweep, retention, quota and drain follow-ups
from PRs 2102 and 2166 (BACKLOG #3184, #3240, #3256)

Proposed banners:
#3184: CLOSED -- items 1, 3, 4 fixed in engine PR <n>; 2 and 5 were done
in d62308d (PR 2166).
#3240: CLOSED -- fixed in engine PR <n>; the Transit-423 low declined.
#3256: CLOSED -- items 1, 3 and the uvicorn drain fixed in engine PR <n>;
items 2 and 4 held by #3180 and #3184.
…decar, one clock read on release

Integration fixes on the combined batch tree (lane B BACKLOG #3191, lane C2
BACKLOG #3184 #3240 #3256), from a review of the merged branch.

- The upload retention runner's stop takes a timeout, and the managed
  lifespan passes it a share of the step's bound (about 2 s with every
  step present), so the runner's own wait ends and cancels the sweep
  before the teardown cuts it. Its 5 s default is now an upper limit.
- The long-operations drain's cancel grace stays below the DR hook reap;
  the budget cannot hold the reap inside the NSSM window, so it is stated
  with "at least" wording in app.py and outlive.py.
- The budget comment names __main__._UVICORN_DRAIN_SECONDS; the NSSM
  window test reads it directly.
- The _TeardownStep docstring names both unbounded steps.
- The quota scan classes a sidecar that fails the whole read again, as
  origin/main's _scan_unsealable_sync did: a plaintext sidecar that is
  not UTF-8 only past the head the class check reads. Test added.
- "cross-shard" in the new upload log lines and the 3256 changelog now
  says engine shards.
- The SQL Server quota release reads SYSUTCDATETIME() once, into
  DECLARE @now: two references in one statement can differ.
Items: BACKLOG #3191, #3184, #3256 (integration fixes on the batch tree).

- A test pins that the managed lifespan hands the retention runner's
  stop a share of its step's bound.
- The quota scan classes a sidecar on a decode error only, not on an
  OSError, so a stalled read is not paid twice under _quota_lock; the
  flag is named for what it means.
- The scan docstring says which non-UTF-8 sidecars are still left out.
- Comments point at the constants instead of restating 2 s and 5 s, and
  the DR reap residual is stated once, at CANCEL_GRACE_SECONDS.
- docs/PHI.md and changelog.d/2264 no longer state a 5 s runner wait.
- The _stop_stuck_runner docstring names both bounds.
…HI.md residual stated

Items: BACKLOG #3184, #3256 (integration fixes on the batch tree).

- uploads.py says engine-shard in every reservation log line and
  comment, one phrasing for the subsystem, in place of the bare
  cross-shard; the 3256 changelog says the same and is reflowed.
- docs/PHI.md qualifies each shard in the uploads row as an engine
  shard, gives the runner's 5 s upper limit with its constant, and says
  which non-UTF-8 sidecars the prune removes and which stay out. It
  says the API cannot delete either kind, which is unchanged from
  origin/main.
- _UnsealableUpload's docstring matches.
- The lifespan test asserts the share is shorter than the runner's own
  default wait.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, BACKLOG #3191 (lane B), head 0a0b923ca22ddb581507f53d2c10e0c0cf7b75cc

  • /simplify ran; applied redundant-None removal, dead sentinel, the sync-step wrapper, the running floor total, the wait-based loop, faster tests and an unused fixture.
  • ruff check and format: clean. mypy four legs: clean (358, 339, 1338, 6).
  • pytest: 8 teardown suites 157 passed; wider lifespan/shutdown/teardown/approval sweep 608 passed, 40 skipped. Full suite not run by the Builder.
  • code-review, level passed xhigh, two rounds. Tag printed: none (JSON only). Round 1: 15 findings, applied via the redesign plus the fixes. Round 2: 13 findings, 6 fixed. Shipped open: see the PR body's new items 1-4.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, BACKLOG #3184, #3240, #3256 (lane C2), head dc580f1b0e1be68a0bf073cf2f29df415ef3cc12

  • /simplify ran (4 agents); applied one-flag scan, store bind/unbind, the _DB_NOW cross-reference, and a trimmed _serve comment.
  • ruff check and format: clean. mypy four legs: clean.
  • pytest: upload suites + test_api_tls 405 passed, 8 skipped; a wider run 5905 passed with only test_connscale_smoke.py failing locally (environmental, serverdb-gated load leg); a 5144-test doc/boundary run passed.
  • code-review, level passed xhigh, two rounds. Tag printed: none (JSON only). Round 1: 13 findings, applied. Round 2: 15 findings, applied. Shipped open: see the PR body's new items 6-10.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA line, batch integration fixes (Builder F), head c7e8f0912bbc454db77502a5643f94de2462697f

  • /simplify: not run (not in this brief).
  • ruff check and format: clean. mypy four legs: clean.
  • pytest: full suite -n 6 was spoiled by the Builder's own mid-run app.py edits (route-walking tests read source by line). Every affected file re-run at final head: 288 passed, 0 failed. Upload, teardown, cross-process quota and PHI inventory tests passed after each round.
  • code-review, level passed xhigh, two rounds, against origin/batch/b261 before the fixes. Tag printed: none. Round 1: 14 findings, applied. Round 2: 10 findings, applied. Shipped open: PR body new items 4-7.
  • Manager, combined tree before F: full pytest -n 6 47,612 passed, 12 failed; the 12 pass serially on main 8ba57eeb and on this branch (81 and 81).

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

Copy link
Copy Markdown
Collaborator Author

Manager b261 (worktree D:\Code\MessageFoundry-manager-batch-next) hands over two more engine PRs.

PR 2363, batch 261 wave: BACKLOG #3191 (PARTIAL), #3184, #3240, #3256 (CLOSED).

  • Head: c7e8f09.
  • Review proof: code-review xhigh, two rounds each, on lanes B and C2 and on the integration fixes. Three QA comments posted, qa label applied.
  • Unread legs: Postgres and SQL Server store legs, windows-service-smoke (the NSSM stop), and connscale smoke.
  • The full suite has not run on the final head locally. On the pre-fix tree, 12 tests failed under -n 6 and all pass serially on main.
  • Ten new items to file are listed in the body.
  • The claims on 3191, 3184, 3240 and 3256 are held by worktree D:\Code\MessageFoundry-b261-batch until merge.

PR 2365: BACKLOG #3181 (rotate-key; it changes a security control, so it has its own PR).

  • Head: 54e1ce0.
  • Review proof: xhigh. QA comment posted, qa label applied.
  • It is STACKED ON 2363: land 2363 first.
  • Four new items to file are in the body. One of them is the serve-start rotation stamp.

Full landing order for batch 261:

  • 2341 then 2352 (the seam stack; re-derive the seam, never pick a side);
  • 2363 then 2365 (the batch stack);
  • the channel scope PR (#3189, #3031) still to come, stacked on 2352. It carries an open owner question.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander: enqueuing at pinned head c7e8f09. QA (xhigh) covers this head, and no finding needs the owner. Every open finding is filed in the vault ledger (#3729-#3736 and notes on #3191 and #3296; vault PR 2494, merged 11:16Z, completeness-reviewed). Required checks are green; the only red, 'ASVS anchors (advisory)', is not required.

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 17, 2026
Merged via the queue into main with commit 064700f Oct 17, 2026
52 of 53 checks passed
@wshallwshall
wshallwshall deleted the batch/b261 branch October 17, 2026 11:53
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