fix(debugger): bound output and make context and cleanup reliable

[xiaozhu1337]

Summary

Closes #130. Fixes #125.

This is a consolidated safety PR: it contains the still-unmerged prerequisite fixes from #126, #127, #128 and #129, plus the newly reproduced fixes below. It is intended as one review/merge alternative to those four PRs, not an independent patch to merge alongside them. They remain open for history; this PR supersedes their combined implementation and corrects the native strict-marker regression in #128.

• Accept exact session-unique completion tokens after genuine native prompts; ignore echoed input, foreign/stale tokens and long-line fragments. Frame markers with a blank .echo to tolerate the native DBCS pipe behavior.
• Require real stopped register context throughout remote command/resume/wait flows; keep one unanswered register probe pending rather than repeatedly queuing it.
• Close when timeout recovery loses synchronization, instead of admitting stale output into a new command.
• Bound retained and returned output to 2,000 lines / 65,536 characters, plus a truncation notice, while still draining transport. Bound single-line reads and preserve a small exit-diagnostic tail.
• Stream Unicode log parsing, quote log paths, rotate completed logs at 512 KiB and close a hung session at the polled 8 MiB safety threshold. This is not an OS-enforced disk quota.
• Retain retryable session ownership on shutdown failure, report constructor-cleanup recovery ids, bound release writes/tree-kill waits and check reader termination. A blocked stdin exit write cannot prevent the fallback kill. Close retries can still apply an updated resume policy.
• Give close/rollback their own worker capacity; move dump discovery off-loop and paginate it with offset/limit (filesystem-order pages, no stable-snapshot guarantee).
• Keep verbose diagnostics off MCP stdout; write errors through a bounded asynchronous queue to rotating local logs.
• Split the oversized session/test modules along process-transport versus command-control responsibilities. No new runtime dependency.

The changes preserve the distinction between closing a user-mode remote client and resuming its independently owned server target. Kernel resume policy remains explicit. Remote clients do not open a server-owned Unicode log.

Tests

On Windows, final PR head 6d1e275130f3ccf04ada46ec0e480eccd26ac532 (runtime fixes in 338a440ab563e30688d2ec5d1fc667ba3ef610d0, followed by test-artifact isolation):

• 232 passed, 6 deselected using pytest src/mcp_windbg/tests -m "not remote and not kernel" -q, supervised at 150 seconds with empty symbol-server paths and only bundled local dumps.
• The opt-in native protocol check passed on both the current WinDbg CDB execution alias and Windows SDK CDB: two local dump cases per executable (Unicode and temporary paths with/without spaces), plus a real localhost dump server/context/independent-client-cleanup case per executable.
• Added regression checks for control-only remote echoes, duplicate probes, lost synchronization, huge/single-line output, Unicode log rotation/oversize cleanup, saturated worker pools, failed tree kills, blocked release writes, constructor recovery ids, retry resume policy and protocol stdout cleanliness.
• git diff --check passes; changed text is UTF-8 without BOM.

uv run python -m pytest "src/mcp_windbg/tests" -m "not remote and not kernel" -q
uv run python "src/mcp_windbg/tests/e2e/check_native_protocol.py"
uv run python "src/mcp_windbg/tests/e2e/check_native_protocol.py" --cdb "PATH_TO_SDK_CDB"

The native check has a 45-second supervisor and kills only its own test process tree. Real running remote targets and kernel targets were not tested, and no production crash reproduction or symbol download was attempted. These wrapper fixes do not prove sole causation of the earlier Windows crashes or guarantee recovery from OS-denied termination/native debugger/kernel faults. Remote DBCS output can still be truncated by native pipe behavior because a remote client does not own the server's log.

[svnscha]

Thank you for the amount of work in here, and for #130, which is a genuinely useful list. I have taken a good deal of it, but not as one branch. Here is the reasoning and exactly what landed.

Why not as one merge

Squash-merging makes this and #126 to #129 mutually exclusive: taking both leaves 7 conflicted files and 17 conflict blocks, so it is a fork rather than an ordering question. Beyond the four PRs it adds roughly +2450/-1440 across 17 files, which is more than I can review to the standard the rest of this backlog got. And a few things in it would have regressed users:

A close followed by a command now waits out the full timeout. server.py:113-115 pops the session only after shutdown(), and _require_session never consults _closing_sessions. A model that calls close_cdb_session and then run_cdb_command on the same id - routine behaviour - sends a command into a dying debugger whose reader thread is gone, so no marker lands and it waits 60 or 120 seconds. Today that answers Unknown session_id immediately. It is a three-line fix, but it was untested.

error_log.py writes target data outside the output filter. It logs full tracebacks, absolute dump paths and partial_output to a hard-coded <package dir>/logs/errors.log, i.e. into site-packages or the uv cache, which is Users-readable on a machine-wide install. Running the suite produced a 28,939 byte file. It bypasses --filter-script, which the README presents as redacting data before it leaves the machine. That one I will not ship in any form; the minimal correct fix for the stdout concern is logging to stderr.

list_dumps changes behaviour for every caller. limit defaults to 50, so a directory of 60 dumps returns 50 (measured over real stdio) while the tool description says nothing about pages. Separately, sorted(glob.glob(...)) became a bare iglob, which changes the result order for everyone, paging or not. That one is not mentioned anywhere in the PR.

?:???> still wedges. debug_output.py:15 promotes LOGGED_PROMPT - a log-scaffolding regex - into the pipe completion gate, where the base used a plain substring. It accepts 0:000>, 1:001:x86>, 8: kd>, lkd>, kd> and a bannered remote prompt, but rejects ?:???> and *BUSY* 0:000>. ?:???> is the no-current-process state, which is exactly a running live target, so open_cdb_remote against one fails where the base works. This is the same shape as the #128 regression, narrower.

Every dump timeout closes the session. Dumps are not is_live_session, so resynced is always false and shutdown() always runs. For someone working a large MEMORY.DMP that costs a reopen and every resolved symbol, even when the command would have cancelled in under a second. #124's cancel-on-dump is not in this branch and is the better answer.

_SHUTDOWN_GRACE_SECONDS = 2.0 against a kd release that needs 1.8s minimum can taskkill kd mid-resume and leave a kernel target frozen.

For what it is worth, the branch does pass the live suite here (236 tests), and server_support.py is clean and packages correctly. The native check does not reproduce though: .echo unicode_<CJK> cannot round-trip on a Western code page, so check_native_protocol.py fails on ACP 1252 and would fail in CI. It is also not pytest-collected, and it fakes the DBCS gate, so #102 is never actually reproduced by it.

What landed from your work

• fix(server): close undisclosed sessions when an open fails #136 - the session-ownership transaction from fix(server): clean up undisclosed sessions when open fails or is cancelled #126, with the rollback changed so it never resumes a live target.
• fix(debugger): make completion markers unique per session #137 - session-unique markers, the correct half of fix(debugger): require session-unique standalone completion markers #128, kept prompt-agnostic.
• fix(server): bound retained output and keep diagnostics off stdout #139 - bounded retained output; the quoted .logopen /u path; a close that no longer lies about success; discovery off the event loop; the torn-read guard on the UTF-16 log.

The quoted log path deserves a specific mention, because it is worse than #130 states. Unquoted, cdb opens a log at the truncated prefix and leaves that file behind, so the #102 channel is silently off for anyone whose temp path has a space in it. Good catch.

Not landing in 1.5: the remote readiness work, deferred to 1.6 with #127, because that path has no automated coverage at all and needs a live running target to test. That absence is why both branches shipped a defect there, and adding the scenario is part of the 1.6 work.

I have also turned on CI for contributor pull requests at the repository level, so future ones get the full matrix without me approving a run. That would have caught the #128 regression on its own, since kernel_dump.yaml opens a committed kernel minidump in CI.

Closing this in favour of those three, with #130 left open for the items not yet taken.