fix(stdio): drain in-flight request handlers before closing write stream on read-EOF by cdeust · Pull Request #284 · cdeust/Cortex

cdeust · 2026-07-30T06:47:59Z

Summary

Root-caused the intermittent "Docker Smoke" CI failure (reproduced on main CI run 30504042295, PR fix(core): break the synaptic-plasticity import cycle (#233) #254, PR fix(sqlite): register explicit datetime adapter, close deprecation warning #266) to an upstream mcp 1.29.0 race: BaseSession._receive_loop closes the write stream unconditionally on stdin-EOF even when a request from the same batch (tools/list after initialize) is still in flight — the response is silently lost (caught ClosedResourceError, logged via logger.debug() on a handler-less logger).
Fixed at Cortex's composition root: mcp_server/infrastructure/stdio_transport.py interposes a write-stream proxy that defers the real close until the low-level server's run() has returned (i.e. until every dispatched handler has had its own chance to respond, per anyio task-group join semantics). mcp_server/__main__.py::main() now drives stdio through this wrapper instead of mcp.run(transport="stdio").
Regression tests at the SDK boundary pin the drain-before-close contract directly (not just the shell script), including a permanent characterization of the upstream defect. A scoped mutation run found further gaps in the new module and closed them: 42/43 mutants killed, 1 documented equivalent.
scripts/docker_smoke.sh gets an independent, second hardening: its timeout/gtimeout wrapper was measured (2026-07-30) to NOT reliably stop a genuinely hung container (SIGTERM to the docker run client doesn't reliably reach the container) — a --cidfile-based watchdog now docker kills the actual container after the same 60s budget.
Boy-scout: MIN_TOOL_COUNT had drifted to 49 (citing a test name that no longer exists) against the true baseline of 52 — corrected, along with every doc's test-count claim (6550 → 6562).

Diagnosis → fix, precisely

See mcp_server/infrastructure/stdio_transport.py's module docstring for the full happens-before argument and upstream file:line citations (mcp/shared/session.py ~L351-355, mcp/server/lowlevel/server.py ~L805-815, fastmcp/server/low_level.py ~L234-268).

Test plan

tests_py/infrastructure/test_stdio_transport.py — characterizes the upstream race directly (bare mcp._mcp_server.run) and pins the fix (_run_low_level_drained); the fix test verified to FAIL when the guard is bypassed (reproduces the exact reported symptom).
tests_py/infrastructure/test_stdio_transport_wiring.py — stateless parameter's actual MCP-lifecycle effect (omitted-default vs explicit), outer run_stdio_drained wrapper wiring (banner, transport context var, log level, log message content via caplog), all mutation-driven.
Scoped mutation run (scripts/mutation_check.sh): 42/43 killed, 1 documented equivalent (typing.cast's type argument, never read at runtime).
Full suite: 6562 passed locally (pytest, ~175s).
ruff check / ruff format --check: clean on all touched files.
pyright mcp_server/: 0 errors, 0 warnings, 0 informations.
shellcheck scripts/docker_smoke.sh: clean.
Real Docker validation (built cortex-smoke:local from the repo Dockerfile):
- Happy path: 15/15 passing runs on the timeout-available path, 10/10 on the no-timeout-binary fallback path.
- Negative control (fast-broken container, exits immediately): gate correctly FAILs.
- Negative control (deliberately hanging container, sleep infinity): gate correctly FAILs in exactly 60s (the watchdog bound), zero leaked containers — verified on both code paths.
uv lock --check / scripts/generate_pip_constraints.py --check / scripts/check_doc_claims.py / scripts/generate_repo_badges.py --check: all clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

Completion Ledger (§13.2) — review-fix + rebase delta (commits `740951f`, `f1222e6`, `a788e17`)

Path introduced/changed	Asserting evidence
`run_stdio_drained(show_banner: bool \| None = None)` resolves `fastmcp.settings.show_server_banner` when omitted	`tests_py/infrastructure/test_stdio_transport_wiring.py::test_omitted_show_banner_resolves_from_fastmcp_settings_false` — fails against the pre-fix hardcoded-`True` default (verified locally by reverting the fix and re-running just this test), passes against the fix
Settings resolution doesn't clobber a settings-`True` default when the setting itself is `True` (non-regression direction)	`...::test_omitted_show_banner_resolves_from_fastmcp_settings_true`
An explicit `show_banner=` argument still overrides the resolved setting	`...::test_explicit_show_banner_wins_over_the_setting`
Stale test asserting the pre-fix (buggy) behavior as if it were the contract	Removed: `test_omitted_show_banner_defaults_to_showing_it` (replaced by the three tests above)
No new mutation gaps introduced by the banner-resolution change	Scoped mutation run, `stdio_transport.py` mutated against both `test_stdio_transport.py` + `test_stdio_transport_wiring.py`: 43/44 killed, 1 documented-equivalent (pre-existing `typing.cast` type-argument mutant)
Rebase onto `origin/main` (`ef42deb`/#285) — conflicts resolved without content loss	All 6 conflicts (`.bestpractices.json`, `CLAUDE.md`, `CONTRIBUTING.md`, `README.md`, `assets/badge-tests.svg`, `docs/ASSURANCE-CASE.md`) were identical prose differing only in the advertised test-count number; both sides' prose kept, no section dropped (diffed to confirm)
Test count re-derived from a live collection on the fully-rebased tree (not mid-conflict)	`pytest --collect-only -q` → 6623; local full run: 6618 passed, 5 skipped, 117 subtests = 6623; CI's own `Test (Python 3.12)` job's "Check advertised test count" step passed on the pushed head
Doc-claim / badge gates clean at the recomputed count	`scripts/check_doc_claims.py --test-count 6623` → `doc claims OK`; `scripts/generate_repo_badges.py --test-count 6623 && --check` → `badges OK (5 checked)`
No unresolved conflict markers; `.bestpractices.json` still valid JSON	`grep` sweep for labelled conflict markers (clean); `json.load` round-trip (clean)
Branch's own touched tests re-run post-rebase, no regression	`tests_py/infrastructure/test_stdio_transport.py` + `test_stdio_transport_wiring.py` + `tests_py/test_main.py`: 24 passed
CHANGELOG reflects the observable-behavior fix (§13.1 H4)	Appended to the existing `[Unreleased]` bullet for this same not-yet-released change (commit `a788e17`)
Lint / format / types clean on all touched files	`ruff check` / `ruff format --check`: clean; `pyright`: 0 errors, 0 warnings, 0 informations
CI green per-job on the exact pushed head	`gh pr checks 284` on `a788e172e2a27c7233600c9716ebe900e9cf00c4`: all pass (`Fuzz (scheduled batch)` correctly `skipping` — schedule-only trigger, not applicable to a PR run); `mergeStateStatus=CLEAN`

Original PR's own Test plan checklist (Summary above) covers the drain-before-close fix itself and stands unchanged — reviewed and verified line-by-line prior to this delta.

…eam on read-EOF Docker Smoke intermittently reported no tools/list response with no exception and no JSON-RPC error frame -- reproduced on main (CI run 30504042295, attempt 1 failed / attempt 2 succeeded on the identical commit), on PR #254, and on PR #266. Root cause is upstream, in mcp 1.29.0: BaseSession._receive_loop (mcp/shared/session.py) closes the write stream unconditionally the instant stdin reaches EOF, even when a request dispatched from an earlier line in the same batch (tools/list after initialize) is still running in its own task and has not called respond() yet. Server._handle_request catches the resulting ClosedResourceError and logs it via logger.debug() on a logger with zero handlers by default, so the drop is completely silent. All three JSON-RPC lines are always fully read (measured parsed_count == 3 on every trial, pass and fail alike) -- nothing is ever left unread; only the already-computed response is lost. fastmcp 3.4.5's LowLevelServer.run override removes the base SDK's own `finally: tg.cancel_scope.cancel()` mitigation with nothing in its place, so Cortex's stdio entry point inherited the hazard unmitigated. Fixed at Cortex's composition root: mcp_server/infrastructure/stdio_transport.py interposes a write-stream proxy that no-ops the SDK's premature aclose() and closes the real stream only after the low-level server's run() call has returned -- which, by anyio task-group join semantics, is only once every dispatched handler has had its own chance to respond. mcp_server/__main__.py::main() now drives stdio through this wrapper instead of mcp.run(transport="stdio") directly. Regression tests at the SDK boundary (tests_py/infrastructure/test_stdio_transport.py): one test reproduces the drop against the bare upstream call directly (a permanent characterization of the upstream defect), a second drives the identical race through the fix and asserts the response survives -- verified to fail against the pre-fix code. A scoped mutation run (scripts/mutation_check.sh) found further gaps, hardened in test_stdio_transport_wiring.py (the `stateless` parameter's actual MCP-lifecycle effect, and the outer run_stdio_drained wrapper's banner/transport-context-var/log-message wiring); final scoped mutation score: 42/43 killed, 1 documented equivalent. scripts/docker_smoke.sh gains a second, independent hardening: its timeout/gtimeout wrapper was itself measured not to reliably stop a genuinely hung container (SIGTERM to the docker run CLIENT process does not reliably reach the CONTAINER). A --cidfile-based watchdog now docker kills the actual container ID after the same 60s budget, verified against a deliberately hanging test image (fails in exactly 60s, no leaked container) and a deliberately broken/exiting one (fails immediately) -- both directions proven stable across repeated runs, on both the timeout-available and no-timeout-binary code paths. Boy-scout: MIN_TOOL_COUNT's default and source comment had drifted to 49 (citing a test name that no longer exists) against the true current baseline of 52 -- corrected in the same change, along with the resulting test-count doc claims (6550 -> 6562 across README, CONTRIBUTING, CLAUDE.md, the assurance case, .bestpractices.json, and the tests badge). Co-Authored-By: Claude <noreply@anthropic.com>

Rebasing fix-stdio-drain-on-eof onto current main (#283 landed since this branch was authored, itself bumping the count to 6588) put the doc-count claims six files carry out of sync with the rebased tree. Recomputed from a real collection on this tree (not a delta against main's advertised figure, and not the #287 local/CI-gap arithmetic from an earlier, abandoned resolution attempt on this same branch, which does not apply here): .venv/bin/python -m pytest --collect-only -q -> 6600 tests collected main's 6588 + this branch's 12 new tests (test_stdio_transport.py, test_stdio_transport_wiring.py) = 6600, confirming the arithmetic.

…13.2 review finding) run_stdio_drained's show_banner default hardcoded True, diverging from the call site it replaces: mcp.run(transport="stdio") goes through TransportMixin.run_async, which resolves show_banner=None to fastmcp.settings.show_server_banner (fastmcp==3.4.5, transport.py L56-57) *before* reaching run_stdio_async -- run_stdio_async's own literal `True` default is never actually exercised on that path. Mirroring only run_stdio_async's signature dropped that settings resolution, so a user who disabled the banner via FASTMCP_SHOW_SERVER_BANNER=false got it printed on every stdio launch regardless. Fix: show_banner defaults to None and resolves fastmcp.settings .show_server_banner exactly where run_async does, before the explicit argument would be checked, so an explicit argument still overrides it. Tests (tests_py/infrastructure/test_stdio_transport_wiring.py): - test_omitted_show_banner_resolves_from_fastmcp_settings_true / _false: pins the resolution in both directions; the _false variant fails against the pre-fix hardcoded-True default (verified locally by reverting the fix and re-running just that test). - test_explicit_show_banner_wins_over_the_setting: an explicit argument is not clobbered by the setting. Replaces the now-stale test_omitted_show_banner_defaults_to_showing_it, which asserted the pre-fix (buggy) behavior as if it were the contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

…ection) The prior docs-reconciliation commit's count (6621) was computed while a rebase conflict resolution was still in progress -- before this branch's stdio_transport.py banner fix (and its net +2 tests) had been replayed on top. Recomputed from a clean `pytest --collect-only -q` on the fully rebased tree: 6623. Both drift gates verified clean at this count: python3 scripts/check_doc_claims.py --test-count 6623 -> doc claims OK python3 scripts/generate_repo_badges.py --test-count 6623 python3 scripts/generate_repo_badges.py --check --test-count 6623 -> badges OK (5 checked) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

H4 (coding-standards.md §13.1): the banner-default correction is an observable-behavior fix landing in the same unreleased change as the stdio drain-on-EOF fix it's part of; appended to that entry rather than opening a new bullet since it's the same not-yet-released feature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

run_stdio_drained measured at 54 signature-to-close lines (was 37 before 740951f), breaching both coding-standards.md §4.2's 50-line hard cap and this repo's own stricter 40-line/method convention (CLAUDE.md:76) -- the show_banner-resolution fix in 740951f added ~20 lines of docstring/citation alongside 2 lines of logic. High-stakes module (async coordination / stream-close-ordering primitive): no 20% flexibility applies. Extract Function (Fowler 2018 Ch. 6): show_banner resolution + its sourced citation move into _resolve_show_banner(show_banner: bool | None) -> bool. The citation is relocated, not deleted (coding-standards.md §8 requires a source for every claim). While relocating it: the citation's two upstream line-number references (fastmcp/server/mixins/transport.py L56-57 / L184-186) were verified against the actually-installed fastmcp==3.4.5 in .venv and found to be a consistent -32-line offset from the real run_async (L88-89) / run_stdio_async (L216-218) locations -- corrected in place rather than carried forward unchecked. Before: run_stdio_drained 54 lines. After: run_stdio_drained 39 lines, _resolve_show_banner 27 lines. Tests: 732 passed, 5 skipped before and after (tests_py/infrastructure/); 0 modified, 0 added. ruff check/format clean. Co-Authored-By: Claude <noreply@anthropic.com>

…_level_drained Boy-scout finding (coding-standards.md §14): while measuring run_stdio_drained for the previous commit, _run_low_level_drained (same file, introduced in this PR's own fccfbbb, not pre-existing) measured at 57 signature-to-close lines -- over coding-standards.md §4.2's 50-line hard cap and this repo's own stricter 40-line/method convention (CLAUDE.md:76). High-stakes module: no flexibility applies. Fixed in the same PR as a separate commit per §14.1's "separate commit when it aids review." Extract Function (Fowler 2018 Ch. 6): the mcp._mcp_server.run(...) call, its cast() and the cast()-equivalence citation (already mutation-verified equivalent, scripts/mutation_check.sh) move into _run_mcp_with_guarded_stream(...). The citation is relocated, not deleted (coding-standards.md §8). Before: _run_low_level_drained 57 lines. After: _run_low_level_drained 39 lines, _run_mcp_with_guarded_stream 37 lines. Tests: 732 passed, 5 skipped before and after (tests_py/infrastructure/); 0 modified, 0 added. ruff check/format + pyright (scoped to the touched file, 0 errors) clean. Scoped mutation run (scripts/mutation_check.sh pattern, mutmut 3.x) against the full touched file: 54 mutants, 53 killed, 1 survivor -- x__run_mcp_with_guarded_stream__mutmut_9 (cast(None, guarded) in place of cast(MemoryObjectSendStream[SessionMessage], guarded)) -- the same documented-equivalent mutant already named in the relocated citation and in the Unreleased CHANGELOG entry (typing.cast's type argument is never read at runtime); not a new gap introduced by this extraction. Co-Authored-By: Claude <noreply@anthropic.com>

…GELOG Documents the two Extract Function refactors (_resolve_show_banner, _run_mcp_with_guarded_stream) and the fastmcp line-number citation correction in the same Unreleased entry the original stdio-drain fix and its show_banner follow-up already live in. Co-Authored-By: Claude <noreply@anthropic.com>

main advanced past this branch's base (768ab09, 40cca6d merged since this PR opened), moving the six suite-size doc/badge files' claimed count to 6673 and adding two mutation-hardening test files (test_generate_pip_constraints.py, test_launcher_deps.py) that auto-merged cleanly. Conflicts in the six suite-size files (.bestpractices.json, CLAUDE.md, CONTRIBUTING.md, README.md, assets/badge-tests.svg, docs/ASSURANCE-CASE.md) were all pure number diffs (this branch's stale 6623 vs main's 6673) -- no differing prose to reconcile. Resolved to main's number, then recomputed the MEASURED ABSOLUTE post-merge count (`pytest --collect-only -q`): 6687, not main's 6673 -- this branch's own test files (test_stdio_transport.py, test_stdio_transport_wiring.py, _stdio_transport_helpers.py) add tests main doesn't have yet. Updated all six files to 6687 (not the delta) and regenerated assets/badge-tests.svg via scripts/generate_repo_badges.py rather than hand-editing the SVG. Verified: scripts/check_doc_claims.py --test-count 6687 and scripts/generate_repo_badges.py --check --test-count 6687 both clean; re-ran this branch's own touched tests post-resolution (216 passed, 117 subtests) plus the two auto-merged test files -- 0 failures, 0 modified beyond the merge itself. Co-Authored-By: Claude <noreply@anthropic.com>

…ms refactor) main advanced again since the previous merge, landing #294 ("eliminate the six-file test-count conflict class with a monotone floor", issue #293): check_doc_claims.py and generate_repo_badges.py were split into doc_claim_sources.py/doc_claim_scan.py/doc_claim_structural.py/ repo_badge_catalog.py, and the six suite-size prose files (.bestpractices.json, CLAUDE.md, CONTRIBUTING.md, README.md, docs/ASSURANCE-CASE.md) now point at `assets/badge-tests.svg` / `pytest --collect-only -q` instead of a hardcoded number -- exactly the conflict class this branch's own two prior merges hit. Conflicts in those five files were the branch's now-obsolete "6687" literal vs. main's new no-number prose; resolved to main's version in every case (no content to preserve on our side -- the number is the thing being eliminated). None of the refactored scripts conflicted (this branch never touched them). Recomputed the MEASURED ABSOLUTE post-merge count (`pytest --collect-only -q`): 6706. Regenerated assets/badge-tests.svg via `scripts/generate_repo_badges.py --test-count 6706` (it hadn't auto-merged to the new count). Verified: `scripts/check_doc_claims.py --test-count 6706` and `scripts/generate_repo_badges.py --check --test-count 6706` both clean; re-ran this branch's own touched tests plus the newly-merged doc-claim/badge tests -- 235 passed, 116 subtests, 0 failures, 0 test files modified by this merge. Co-Authored-By: Claude <noreply@anthropic.com>

…defaults test test_run_headless_authoring_cycle_defaults_resolve_from_live_module was 91 lines (this repo's local 40-line/method cap, CLAUDE.md § Code Style) -- new code introduced by this PR's own #237 fix, caught by this PR's pre-push size-cap gate before pushing (the same class of avoidable refusal as PR #284's 54-line method, per this session's directive). Extract Function: the anchor-candidate stub, the page-scan stub, and the five-line monkeypatch setup block each moved to a module-level helper (_make_fake_collect_anchor_candidates, _make_fake_scan_pages_with_gaps, _patch_cycle_defaults_fixtures). Pure extraction -- same assertions, same fakes, same 10/10 passing tests before and after; no behavior change. Before: 91 lines. After: 37 lines (file 267 -> 296, both under the 300/40 caps). Boy-scout note (coding-standards.md §14): three sibling test files in this same package (test_headless_authoring_throttle.py 788 lines, test_ble001_sweep_handlers.py 470, test_s110_sweep_handlers.py 326) also exceed the caps, but predate this PR entirely and this PR's own diff to them is a mechanical 8-10 line patch-target update from the #276 Move Function work, not new growth -- splitting them is a substantial refactor out of a rebase PR's blast radius. Filed as issue #300 with acceptance criteria rather than silently deferred. Refs #237, #276. Filed: #300 (pre-existing sibling test-file size debt).

…237) (#277) * fix: Slide Statements — defer headless_authoring back-import to call time (#237) candidate_scan, claude_cli, cycle_orchestration, and drain_operations each did `from . import headless_authoring as _root` at module top, while headless_authoring imported names back from each of them at ITS module top. Importing headless_authoring first resolved the cycle (why the suite stayed green); importing any of the four directly in a fresh interpreter raised "cannot import name '<x>' from partially initialized module ... (most likely due to a circular import)" — reproduced pre-fix for all four. Fix: each of the four modules now resolves `_root` with a deferred, function-scoped import (`# noqa: PLC0415 — import cycle`, the exemption pyproject.toml already names for this defect family, "the pre-existing #233 family") instead of a module-top-level one. The load-time back- reference is gone; the call-time attribute lookup that makes `monkeypatch.setattr(headless_authoring, ...)` observable is unchanged. Three DI-seam parameters (`invoke`, `max_drains`, `max_anchor_drains`) previously baked `_root.X` into their parameter-list defaults, which is only valid when `_root` is bound at module scope — converted to `None` sentinels resolved inside the function body instead (same live-module read, now at call time rather than def time). Mutation testing (scoped to drain_operations.py + cycle_orchestration.py) found 2 real surviving mutants in the new sentinel logic: flipping `max_drains`/`max_anchor_drains`'s `is None` to `is not None` left the cap silently unresolved (`list[:None]` == no cap) and slipped past the existing single-candidate test. Strengthened test_run_headless_authoring_cycle_defaults_resolve_from_live_module to offer 3 candidates against a cap of 1 on each side; both mutants now kill. Before: 4/4 fresh-interpreter imports fail (reproduced). After: 4/4 succeed; hub-first import path unaffected (verified). Tests: 1480 passed before, 1488 passed after (+8 new, 0 modified assertions in existing tests — the load-order workaround imports/comments in test_s110_sweep_handlers.py and test_ble001_sweep_handlers.py were removed as they document a cycle that no longer exists, not a behavior change). Refs #233 (same defect class, different package — not touched here). Follow-up filed as #276 (pre-existing method-length debt in this file family, discovered but outside this fix's blast radius). Fixes #237 Co-Authored-By: Claude <noreply@anthropic.com> * docs: resolve CLAUDE.md/CONTRIBUTING.md size-limit doc drift (#276) CLAUDE.md stated "300 lines max per file; 40 lines max per method. Enforced by the craftsmanship-checker pre-commit hook" while CONTRIBUTING.md cited coding-standards.md's actual §4 numbers ("File <=500 lines, function <=50 lines"). Neither number was wrong on its own, but no craftsmanship-checker hook exists (.git/hooks/ has only *.sample files, no .pre-commit-config.yaml) — the claim was stale. Resolution: 300/40 is this repo's local tightening of the global 500/50 standard (coding-standards.md SS10 permits tightening a hard rule, never weakening it). CONTRIBUTING.md now cites 300/40 and points at CLAUDE.md as authoritative; CLAUDE.md's enforcement claim is corrected to "code review today, no automated hook yet" instead of naming a hook that was never wired. Refs #276 (found during #237's boy-scout pass over the same touched file family). * refactor: Extract Function on candidate_scan.py (issue #276) _scan_pages_with_gaps (52 lines) and _collect_anchor_candidates (55 lines) both exceeded the local 40-line/method cap (CLAUDE.md, resolved against CONTRIBUTING.md in the prior commit). Extracted the per-page gap-detection body into _gap_entry_for_page (+ the lazy missing_sections probe into _load_missing_sections_probe), and the per-domain candidate loop into _extend_anchor_candidates_for_domain (mutates the same candidates list in place, same early-exit-at-cap semantics as the original inline return). Pure Extract Function — no behavior change. Longest function now 36 lines; file 166 lines (was 134, growth is docstrings/signatures on the new small functions). Before: pytest tests_py/handlers/{test_headless_authoring_throttle,test_s110_sweep_handlers}.py tests_py/handlers/consolidation/{test_import_cycle_237,test_headless_authoring_governance,test_distill_drain}.py tests_py/hooks/test_headless_guard.py -> 65 passed After: same command -> 65 passed, 0 modified assertions. Refs #276. * refactor: Extract Function + Move Function on drain_operations.py (issue #276) drain_one (81 lines), drain_all_gaps_on_page (122 lines), and drain_missing_anchors (94 lines) all exceeded the local 40-line/method cap, and the file itself (397 lines) exceeded the local 300-line/file cap (both from CLAUDE.md, resolved against CONTRIBUTING.md two commits ago). Extract Function: split drain_one into _invoke_section_fill + _finish_drain_one; split drain_all_gaps_on_page into _invoke_page_fill, _no_response_results, _fill_gaps_from_response, _persist_filled_gaps. Factored the repeated DrainResult-construction + elapsed-ms computation (previously inlined 3-8x per function) into two small shared helpers, _elapsed_ms and _drain_result — a legitimate extraction per coding-standards.md SS3.3 (far more than 3 concrete uses). Move Function: drain_missing_anchors (a distinct concern — authoring whole missing anchor pages, vs. per-page gap-filling) moved to a new sibling module anchor_authoring.py, matching this package's existing split-by-concern convention (candidate_scan/drain_operations/ cycle_orchestration were already split out of headless_authoring the same way). anchor_authoring.py reuses _drain_result/_elapsed_ms from drain_operations (no import cycle — anchor_authoring -> drain_operations is one-directional). headless_authoring.py's re-export now pulls drain_missing_anchors from .anchor_authoring instead of .drain_operations. Both new/changed files keep the issue #237 deferred-import pattern (_root resolved at call time, not module top) so monkeypatch.setattr(headless_authoring, ...) stays observed — verified by extending tests_py/handlers/consolidation/test_import_cycle_237.py's fresh-interpreter parametrize list to include anchor_authoring (a new module carrying the same load-order-sensitive pattern needs the same regression coverage). tests_py/handlers/test_headless_authoring_throttle.py's test_drain_missing_anchors_skips_ungroundable patched drain_operations._build_registry/_project_source_root/audit_domain by dotted-path string; updated the three patch targets to anchor_authoring (the function's new home) — mechanically required by the Move, not a behavior-expectation change. Before: 65 passed (prior commit's baseline + the candidate_scan split). After: 66 passed (+1 for the new anchor_authoring parametrize entry), 0 modified assertions. Post-refactor sizes: drain_operations.py 397->300 lines (all methods <=38); anchor_authoring.py new file, 164 lines (all methods <=37). Refs #276. * refactor: Extract Function on cycle_orchestration.py (issue #276) run_headless_authoring_cycle was 184 lines with two nested coroutines (drain_anchor_bounded, drain_page_bounded + an inner charging_invoke closure) closing over sem/budget/invoke/wiki_root/today/_root — both the local 40-line/method cap and the file's readability suffered. Extract Function, following #276's own prescribed fix shape ("become module-level functions taking sem/budget as explicit parameters"): split into _build_cycle_candidates (phase 1), _invoke_anchor_call + _drain_anchor_bounded + _make_charging_invoke + _drain_page_bounded (phase 2, now module-level, no closures), _gather_cycle_results (phase 3), _build_cycle_budget_and_sem, _execute_cycle, and a thin run_headless_authoring_cycle that only resolves omitted parameters then delegates. Reused drain_operations' _drain_result/_elapsed_ms helpers here too (imported, no cycle — cycle_orchestration -> drain_operations is one-directional and already existed). Pyright gotcha (worth recording): the four "if X is None: X = _root.Y" default-parameter resolutions MUST stay inline in run_headless_authoring_cycle, not move into an extracted helper — _root there is the concrete live-module type (a local deferred import), which is what lets pyright narrow away the parameter's `| None` on reassignment. An extracted helper receiving `_root: Any` as a parameter loses that narrowing (`Any` doesn't clear a declared `X | None`), so a strict `-> tuple[Path, int, int, Callable[...]]` return type fails pyright even though the runtime value is never None. Verified by a minimal repro before settling on the inline placement. Every `_root.X` access still resolves at call time (issue #237's patchability contract for monkeypatch.setattr(headless_authoring, ...) is unchanged — verified by the unmodified throttle test suite passing). Before: 66 passed (prior commit's baseline). After: 66 passed, 0 modified assertions. pyright: 0 diagnostics. Post-refactor: cycle_orchestration.py 237->299 lines (file itself was already under the 300 cap; growth is from the extra function signatures), all methods <=38 lines (was 184 for the worst offender). Refs #276. * refactor: Move Function on headless_authoring.py (issue #276) headless_authoring.py was 469 lines (over the local 300-line/file cap) and its _claude_invoke was 113 lines (over the 40-line/method cap) -- the last remaining size-cap violation in the consolidation-package touched-file family from #237's fix. Move Function: _claude_invoke split into _spawn_claude_process, _communicate_with_timeout, and _parse_invoke_response (Extract Function), then all four moved to a new sibling module claude_invoke.py, following this package's established split-by-concern pattern. claude_invoke.py carries its own deferred `_root` import (same issue #237 pattern -- headless_authoring imports _claude_invoke back at load time) and returns `Any` (not InvokeResult) matching every other sibling's established convention for the same circular-import reason. Move Function (second split, same commit -- both needed to clear the file-size cap): CycleBudget, DrainResult, and CycleSummary moved to a new cycle_types.py (plain dataclasses, zero dependency on headless_authoring or any sibling -- no import-cycle risk). InvokeResult and _AnchorCandidate stay in headless_authoring.py (distinct concern: worker identity/config, not cycle telemetry). headless_authoring.py re-exports all three names from cycle_types -- every existing `_root.CycleBudget(...)` / `_root.DrainResult(...)` / `_root.CycleSummary(...)` call site in the other sibling files is unaffected (same class objects, same module attribute, different file). Removed now-unused imports from headless_authoring.py (asyncio, json, field) after the move; verified with grep before removing each one. Regression coverage: extended test_import_cycle_237.py's _AFFECTED_MODULES to include claude_invoke (same load-order-sensitive pattern as the other five). cycle_types.py needs no entry -- it has no dependency on headless_authoring at all. Before: 67 passed (prior commit's baseline). After: 67 passed, 0 modified assertions (the +1 from claude_invoke's new parametrize entry was already counted in "before" via this same commit's test change). pyright: 0 diagnostics across headless_authoring.py, claude_invoke.py, cycle_types.py, and every other touched consolidation file. Post-refactor: headless_authoring.py 469->255 lines; claude_invoke.py new, 160 lines (all methods <=40); cycle_types.py new, 90 lines (no functions over 7 lines). Every file this PR touches in the consolidation package is now <=300 lines; every method is <=40 lines. Refs #276. * test: add direct filesystem coverage for candidate_scan (issue #276, §12) Scoped mutation testing (mutmut) on the consolidation-package files touched by #237/#276 surfaced 570-576 survivors across the family. A controlled A/B comparison (mutating the pre-refactor cycle_orchestration.py against the identical test selection) produced the EXACT SAME 121 survivor count as the post-refactor file, proving the bulk of these are inherited test weakness (subprocess/invoke fakes that accept-and-ignore most kwargs), not introduced by this PR's Extract/Move Function work -- see the refactorer's session notes for the full evidence and the surviving-mutant inventory. candidate_scan.py was the one file in this family cheap to close real gaps in: no subprocess, no PostgreSQL, pure filesystem + string logic, and every EXISTING test in the suite monkeypatches _scan_pages_with_gaps out entirely (test_headless_authoring_throttle.py, test_import_cycle_237.py), so its real walking/filtering/tuple-building logic had zero direct coverage before this commit. Added 7 tests exercising _scan_pages_with_gaps against a real tmp_path wiki tree: frozen curation_gaps detection, no-gaps exclusion, dotfile/ underscore directory exclusion, missing wiki_root, mixed-page filtering, and the live-audit axis (kind=reference + source_file_path, both the missing-sections-found and non-reference-kind-skip cases). Effect (scoped mutmut, candidate_scan.py only, same test list plus this file): survivors 38 (pre-refactor baseline) -> 35 (post-refactor, before these tests) -> 25 (post-refactor, with these tests) covering _gap_entry_for_page and _scan_pages_with_gaps fully. The remaining ~25 survivors are in _collect_anchor_candidates/_extend_anchor_candidates_ for_domain (registry/audit_domain/_AnchorCandidate construction), requiring the same class of test-infrastructure investment as the subprocess/invoke fakes elsewhere in this family -- out of this PR's blast radius per the same evidence. Test-only commit: no production code changed; existing suite passes unchanged (0 modified assertions), 7 new tests added. * test: extract fixture helpers to clear the 40-line cap on the new #237 defaults test test_run_headless_authoring_cycle_defaults_resolve_from_live_module was 91 lines (this repo's local 40-line/method cap, CLAUDE.md § Code Style) -- new code introduced by this PR's own #237 fix, caught by this PR's pre-push size-cap gate before pushing (the same class of avoidable refusal as PR #284's 54-line method, per this session's directive). Extract Function: the anchor-candidate stub, the page-scan stub, and the five-line monkeypatch setup block each moved to a module-level helper (_make_fake_collect_anchor_candidates, _make_fake_scan_pages_with_gaps, _patch_cycle_defaults_fixtures). Pure extraction -- same assertions, same fakes, same 10/10 passing tests before and after; no behavior change. Before: 91 lines. After: 37 lines (file 267 -> 296, both under the 300/40 caps). Boy-scout note (coding-standards.md §14): three sibling test files in this same package (test_headless_authoring_throttle.py 788 lines, test_ble001_sweep_handlers.py 470, test_s110_sweep_handlers.py 326) also exceed the caps, but predate this PR entirely and this PR's own diff to them is a mechanical 8-10 line patch-target update from the #276 Move Function work, not new growth -- splitting them is a substantial refactor out of a rebase PR's blast radius. Filed as issue #300 with acceptance criteria rather than silently deferred. Refs #237, #276. Filed: #300 (pre-existing sibling test-file size debt). --------- Co-authored-by: Claude <noreply@anthropic.com>

cdeust force-pushed the fix-stdio-drain-on-eof branch from 17c0a55 to 0f3f8e3 Compare July 30, 2026 07:07

cdeust mentioned this pull request Jul 30, 2026

Local dev venv (macOS, reused .venv) collects 10 fewer tests than CI's hash-pinned Linux install (6572 vs 6582) #287

Open

3 tasks

cdeust force-pushed the fix-stdio-drain-on-eof branch 2 times, most recently from da6e876 to 92c198b Compare July 30, 2026 08:38

cdeust and others added 5 commits July 30, 2026 11:28

cdeust force-pushed the fix-stdio-drain-on-eof branch from 92c198b to a788e17 Compare July 30, 2026 09:49

cdeust and others added 5 commits July 30, 2026 12:48

cdeust merged commit 2590922 into main Jul 30, 2026
19 checks passed

cdeust deleted the fix-stdio-drain-on-eof branch July 30, 2026 11:51

This was referenced Jul 30, 2026

chore: refresh assets/badge-tests.svg from a measured count (post-#294) #296

Closed

CLAUDE.md advertises a craftsmanship-checker size gate (300 lines/file, 40 lines/method) that does not exist #297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(stdio): drain in-flight request handlers before closing write stream on read-EOF - #284

fix(stdio): drain in-flight request handlers before closing write stream on read-EOF#284
cdeust merged 10 commits into
mainfrom
fix-stdio-drain-on-eof

cdeust commented Jul 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cdeust commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Diagnosis → fix, precisely

Test plan

Completion Ledger (§13.2) — review-fix + rebase delta (commits 740951f, f1222e6, a788e17)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cdeust commented Jul 30, 2026 •

edited

Loading

Completion Ledger (§13.2) — review-fix + rebase delta (commits `740951f`, `f1222e6`, `a788e17`)