fix(cli): emit ASCII-safe JSON so results survive a legacy stdout codec

[Frank Li (franklixuefei)]

Closes#342.

The bug

On Windows under a cp1252 locale, conductor run crashed with UnicodeEncodeErrorafter the workflow had already completed successfully, while writing its final JSON result to stdout. The document was truncated mid-field so no caller could parse it, and the process exited non-zero, so automation concluded a successful run had failed. When it hit me, a 55-minute, $6.17 run looked like total data loss — the outputs were safe on disk, but nothing in the exit status or the dashboard said so.

Root cause

json.dumps already emits pure ASCII by default, so conductor hands off a safe string. What re-introduces the unencodable character is rich: Console.print_json defaults to ensure_ascii=False and re-parses/re-serialises the payload with the literal glyph immediately before the write. The JSON channel is made unsafe by the pretty-printer, not by our serialisation — that diagnosis is Jason Robert (@jrob5756)'s, and it moved the fix somewhere I would not have looked.

Two details explain the artefact's shape:

• Truncated rather than empty — rich's _write_buffer chunks writes at 8192 chars onlyif WINDOWS (a CPython #82052 workaround). The ASCII chunks land; the chunk containing the glyph dies.
• Strictly after success — CPython gives sys.stderrerrors='backslashreplace' (never raises) while sys.stdout gets strict under a non-UTF-8 locale, so 27,411 lines of progress output got through and only the final stdout write died.

The change

ensure_ascii=True at all five JSON sinks — cli/app.py ×4 and cli/doctor.py (the data= kwarg form).

Lossless by construction: \uXXXX escapes (surrogate pairs for non-BMP) are valid JSON and decode back to the original text.

Two alternatives were considered and rejected:

• errors="replace" stops the crash but silently turns agent review feedback into ?.
• errors="backslashreplace" emits \U0001f600 for non-BMP, which is not valid JSON — JSON requires surrogate pairs.

Tests

CI runs on ubuntu-latest, so these simulate the platform rather than detect it — anything gated on sys.platform would silently never run. Each behavioural test renders through a TextIOWrapper bound to strict cp1252 and asserts both that nothing raises and that the payload round-trips through json.loads, which is the property a machine consumer actually depends on.

Three things worth calling out, because the first version of these tests was weaker and a review caught it:

A production-source guard. The behavioural tests construct their own Console, so on their own they would still pass if someone deleted ensure_ascii=True from the call sites — a regression guard that cannot fail on the regression. An AST assertion over cli/app.py and cli/doctor.py now requires every print_json call to pass it. Verified against a simulated revert (fails, reporting app.py:553,566,1008,1014) and against the fixed tree (passes). It also catches the likelier future regression: a new sink added without the keyword.

A fixture guard. U+2014 EM DASH is deliberately excluded from the samples because cp1252 can encode it (0x97), so it would make its cases pass vacuously. test_samples_are_genuinely_unencodable asserts each retained sample really does raise, and pins the EM DASH exclusion so the reasoning cannot rot. That asymmetry is also why this looked intermittent in the field — whether it fired depended on which glyph an agent happened to emit.

A negative control, so a future rich release that changed its own default cannot leave the suite passing for a reason unrelated to conductor's code.

Deliberately not in this PR

Agreed with Jason Robert (@jrob5756) on the issue:

• CLI-entrypoint stream reconfigure as defence in depth. Worth doing, but it belongs in __main__.py + the app() callback rather than conductor/__init__.py — that module is an advertised library entry point, so reconfiguring there would mutate process-global streams for any embedding host (pytest, ipykernel). Separate PR.
• PYTHONUTF8=1 in _build_bg_env. Once the JSON sink is ASCII this is no longer load-bearing, and it is broader than it looks: cli/pid.py reads and writes PID files with no explicit encoding, so making the child UTF-8 while conductor stop stays on the locale codec would split-brain them — and UnicodeDecodeError is a sibling of JSONDecodeError under ValueError, so the existing except (json.JSONDecodeError, OSError) would not catch it. Wants its own change with pid.py encodings pinned in the same commit.
• Human-prose paths (gates/human.py, gates/dialog.py) still render literal Unicode to stdout. That is out of scope here — it needs stream-level hardening, not JSON escaping — and I am not claiming universal Unicode safety.

Known gap

These unit tests do not exercise rich's Windows-only 8192-char chunked-write path, so they cannot catch a truncation regression specifically. The right instrument is an end-to-end subprocess test running the CLI with PYTHONIOENCODING=cp1252 and asserting json.loads(proc.stdout), which would run green on Linux CI. I have not added it here — happy to fold it in if you would rather it landed together.

Verification

Ran on Windows with the affected interpreter. Without the change, all four samples raise UnicodeEncodeError on a strict cp1252 stream; with it, all four emit pure-ASCII documents that round-trip losslessly, including the data= kwarg form used by doctor.

One correction to the issue's environment table, since it sent you looking: the workflow ran on the uv-managed Python 3.14.5 (so requires-python >=3.12 is satisfied and there is no hidden install bug). The 3.10.11 I quoted was the ambient interpreter I used for the standalone repro. Both give cp1252, so the repro stands.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@9afec0a). Learn more about missing BASE report.

Additional details and impacted files

@@ Coverage Diff @@## main #381 +/- ##
=======================================
Coverage ? 91.80% =======================================
Files ? 112 Lines ? 18239 Branches ? 0 =======================================
Hits ? 16744 Misses ? 1495 Partials ? 0 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jason Robert (jrob5756)]

Thanks for sending this, Frank. Verified the fix end-to-end before reading it. On main, PYTHONIOENCODING=cp1252 conductor --silent run with a non-cp1252 glyph in the output exits 1 with zero bytes on stdout. On this branch it exits 0 and round-trips losslessly, surrogate pairs included. The diagnosis is right and the five sinks are the right five. I also checked the rich>=13.0.0 floor, since ensure_ascii only reached print_json in 12.6.0, and an isolated 13.0.0 install accepts it.

Three things I would want before this lands.

conductor doctor still crashes on cp1252 in its default table mode, in a file this PR edits, because of hardcoded glyphs at doctor.py:36-39. Comment inline.

The module docstring on the new test file describes the version that had the CliRunner tests, and it is no longer true of anything in the file.

Nothing asserts that the fix works. I mutation-tested each of the five sinks against the full suite: every revert fails exactly one test, and it is always the AST source-text guard. The comment explaining why a behavioural test was infeasible is not accurate, since test_run.py:1034 already reaches one of these sinks with a mock, and a working version takes about ten lines and 1.5 seconds.

Smaller items are inline: a misattributed warning in the terminate handlers that survives this fix, a new ty error, and a cluster of now-inert test scaffolding left behind by the CliRunner removal.

Three that do not fit on a line.

The five repeated keywords would be better as one helper. This repo consistently puts that kind of invariant behind a single entry point, with normalize_agent_output and _run_set_step as the closest analogues, and cli/ already has print_error and the display_* family. One helper makes new sinks safe by construction, collapses the two mirrored try/except blocks, and lets the guard become a real test rather than an assertion about source text.

No CHANGELOG entry. The [Unreleased] / Fixed section is active, and this is visible on every platform rather than just Windows, since results now carry \uXXXX escapes everywhere.

Two worth filing rather than fixing here. conductor run | head on a successful workflow exits 1 with nothing on stderr, because rich's on_broken_pipe raises SystemExit and escapes both handlers. And under --silent the event-log path that actually holds the results is printed through _verbose_console, so it is suppressed in precisely the mode where someone needs it.

The fixture self-guard is the best thing in the PR. Pinning the em-dash exclusion so the reasoning cannot rot is a nice touch, and assert calls catching sink removal is a property most AST guards miss.

[Jason Robert (jrob5756)]

This fixes --json, but the default table path still dies on the same stream. doctor.py:36-39 hardcodes \u2713, \u2717 and \u25cb, and cp1252 cannot encode any of them.

On this branch:

$ PYTHONIOENCODING=cp1252 conductor doctor
EXIT=1
stdout: 395 bytes, the Environment table and nothing after it

_render_env and _render_providers are separate console.print() calls, so stdout is left truncated mid-report. That is the same shape as #342, and it lands on the person who ran doctor because they hit #342 in the first place.

The AST guard lists conductor.cli.doctor, which reads as though the module is now covered. Either handle the glyphs here, or say in the description that table mode is out of scope and open a follow-up.

[Jason Robert (jrob5756)]

Nothing at the call site says why this keyword is here, and next to json.dumps it looks provably redundant, since that already escapes. The next person to read the line has a fair reason to delete it. The AST guard would catch that, but it lives in another tree, and the reasoning currently exists only in a test file and a commit message.

Suggested change

	# Output as JSON to stdout
	output_console.print_json(json.dumps(result))
	output_console.print_json(json.dumps(result), ensure_ascii=True)
	# `ensure_ascii=True` is not redundant with `json.dumps`: rich re-parses
	# the string and re-serialises it with `ensure_ascii=False`, restoring
	# literal glyphs that a cp1252 stdout cannot encode (issue #342).
	output_console.print_json(json.dumps(result), ensure_ascii=True)

The other four sinks can point back here rather than repeat it.

[Jason Robert (jrob5756)]

UnicodeEncodeError subclasses ValueError, so this clause was already catching the #342 crash on the two terminate paths. It reported it as a serialisation failure and exited 1, which a failed terminate does anyway, so the regression never showed up in an exit code at all.

Your change removes that trigger, but the handler still misreports what is left. With a closed stdout it prints could not serialise terminate output: I/O operation on closed file, when serialisation succeeded and the write failed. Given json.dumps defaults to ensure_ascii=True and everything here round-trips through _maybe_parse_json, the named TypeError case is close to unreachable while the reachable causes are all stream I/O.

Splitting by stage rather than by exception type fixes the message for good, and keeps the json.dumps guard the comment above asks for:

try:
payload=json.dumps(e.output, default=str)
except (TypeError, ValueError) asjson_exc:
logger.exception("Failed to serialise terminate output")
console.print(
f"[yellow]Warning:[/yellow] could not serialise terminate output: {json_exc}"
)
else:
try:
output_console.print_json(payload, ensure_ascii=True)
except (OSError, ValueError) aswrite_exc:
logger.exception("Failed to write terminate output to stdout")
console.print(
f"[yellow]Warning:[/yellow] terminate output could not be written: {write_exc}"
)

Same applies at app.py:1015. Not blocking, but you are already editing the line above it.

[Jason Robert (jrob5756)]

This describes the version that had the CliRunner tests. As it stands nothing in the file imports output_console, patches a console, or exercises the data= shape.

I checked the load-bearing claim directly by reverting each sink one at a time and running the full suite. Every revert fails exactly one test, the AST guard, and never a corresponding behavioural one.

This docstring is what a maintainer reads when deciding whether the coverage here is enough, which is why I would not leave it. Either make it true, per my note further down, or rewrite it to describe what the file does now: a contract test, a negative control, and a source guard.

[Jason Robert (jrob5756)]

This is why the file ended up with no behavioural coverage, and I do not think it holds. tests/test_cli/test_run.py:1034 already reaches app.py:566 by patching conductor.cli.run.run_workflow_async. No subprocess, no workflow execution. What the CliRunner attempt was missing was that mock, not the process boundary.

I wrote it out and it works:

withpatch("conductor.cli.run.run_workflow_async") asmock_run, \
patch("conductor.cli.app.output_console", con): # con writes to strict cp1252mock_run.return_value= {"summary": "done \u2192\u2705"}
result=runner.invoke(app, ["run", str(wf)])
assertresult.exit_code==0assertjson.loads(raw.getvalue().decode("cp1252"))["summary"] =="done \u2192\u2705"

Two tests, 1.49s, and they discriminate per sink: reverting app.py:553 fails only the success test, reverting :566 fails only the terminate one. All five sinks already have CliRunner tests in the suite, they just run over a UTF-8 stream, so the scaffolding exists five times over.

doctor.py:94 is the one most worth covering, because data= goes through JSON.from_data rather than the string path the negative control exercises. I confirmed that path is independently vulnerable.

The subprocess test should not block. I measured about 5.5s per CLI invocation, and once the above is in, its remaining value is the Windows chunked-write truncation path and stdout pollution.

[Jason Robert (jrob5756)]

text() has no caller now, which leaves self._raw write-only. The arguments around it went the same way: newline="" and force_terminal=False only mattered while something decoded the buffer, and width=200 never did anything, because print_json ends with soft_wrap=True.

I removed each in turn and the file stayed at 7 passed. Strip them and _Cp1252Console is two lines with one caller, at which point it may as well be inline. The pair to keep is encoding="cp1252" and errors="strict", which is what makes the assertion real.

The better direction is the opposite one: add the behavioural test and text() earns its place back, along with the non-BMP sample, whose surrogate-pair behaviour nothing currently exercises.

[Jason Robert (jrob5756)]

ParameterSet.values is typed Sequence[object], so this trips ty with Object of type object has no attribute encode. It also reaches into _pytest.mark.structures, which is not part of pytest's public surface. CI only gates src, so it will not turn the build red, but AGENTS.md asks for type hints that check clean.

Both problems go away if the samples start life as a mapping:

Suggested change

	forparaminNON_CP1252_SAMPLES:
	char=param.values[0]
	withpytest.raises(UnicodeEncodeError):
	char.encode("cp1252")
	forcharin_UNENCODABLE.values():
	withpytest.raises(UnicodeEncodeError):
	char.encode("cp1252")

with _UNENCODABLE: dict[str, str] holding the id-to-character pairs and NON_CP1252_SAMPLES = [pytest.param(c, id=i) for i, c in _UNENCODABLE.items()] above it.

[Jason Robert (jrob5756)]

The comment above says this catches a new sink added without the keyword, which is the likelier future regression. It only does so inside these two modules. I added an unguarded print_json to cli/checkpoint.py, which already owns a stdout output_console, and all 7 tests stayed green. A --json flag on checkpoint list is a plausible next sink.

Discovering them is roughly the same amount of code:

_SRC=Path(conductor.__file__).parent_JSON_SINK_MODULES=sorted(
"conductor."+p.relative_to(_SRC).with_suffix("").as_posix().replace("/", ".")
forpin_SRC.rglob("*.py")
if"print_json"inp.read_text(encoding="utf-8")
)

Two smaller gaps in the matcher itself. _print_json_calls requires ast.Attribute, so if these five sites ever move behind a shared helper called as a bare name, the guard silently stops matching. And {kw.arg: kw.value for kw in call.keywords} maps a **kwargs splat to None, so print_json(x, **opts) would report as missing the keyword.

It is also blind to a Console subclass that overrides print_json, which run.py:50-53 explicitly anticipates. A behavioural test is immune to all three.

[Jason Robert (jrob5756)]

Narrower than it reads. LC_ALL=C is also a non-UTF-8 locale and gets surrogateescape, so someone reproducing on Linux that way sees mojibake and no crash, and may conclude the harness is unrealistic. I measured both.

The rot is the part I would flag. PEP 686 makes UTF-8 mode the default in 3.15, which forces surrogateescape everywhere. Because the wrapper hardcodes strict, the tests keep passing while this sentence quietly stops being true, with nothing to catch it. Pinning it to Windows and a legacy code page would age better.

The sys.stderr parenthetical is correct and genuinely useful, so I would keep that half as is.

[Frank Li (franklixuefei)]

Thanks — the "nothing asserts the fix works" finding was the important one, and it was correct. Addressed in 7d5f135.

The missing behavioural coverage

You were right on both counts: reverting any of the five sinks failed only the AST guard, and the comment claiming a behavioural test was infeasible was wrong. tests/test_cli/test_run.py already reaches app.py's success sink by patching conductor.cli.run.run_workflow_async — what my earlier attempt was missing was that mock, not the process boundary.

TestRealSinksThroughTheCli adds it, essentially as you wrote it. Verified per sink rather than asserted:

	before	after
revert app.py success-sink ensure_ascii	1 failure (AST guard only)	2 failures — guard and the behavioural test

The doctordata= shape has its own case, for the reason you gave: JSON.from_data rather than the string path the negative control exercises, so it is independently vulnerable.

text() earns its place back, along with the _Cp1252Console arguments — I left them rather than stripping them, since the behavioural tests now decode the buffer.

conductor doctor table mode

Confirmed, and confirmed precisely — \u2713, \u2717 and \u25cb all raise under cp1252, while \u2014 encodes at 0x97, exactly as you had it.

I started fixing it here and stopped. A correct fix selects glyphs from the console's stream encoding, which means threading the console through ~18 call sites across _render_env, _render_providers and six _*_cell helpers, none of which currently take one. That is a bigger change than this PR, and a half-fix would be worse than either option.

So I took your second option, deliberately and visibly: filed as #401 with the repro and a suggested shape, and said so in three places — the glyph definitions, the module docstring, and the CHANGELOG — so the AST guard's mention of conductor.cli.doctor reads as "its JSON sink is covered" rather than the whole module.

Smaller items

• Docstring rewritten. It described the version that had the CliRunner tests. It now says what the file does, and states plainly that behavioural coverage was absent before this commit rather than quietly becoming true again.
• ty error fixed. The samples start life as a dict[str, str] and the params are built from it, so nothing reads ParameterSet.values or reaches into _pytest.mark.structures.
• CHANGELOG entry added — the PR had none.

Deliberately not done here

• The five-keyword helper. I agree with the reasoning, and it is the better shape. I left it because the behavioural tests now make the guard a real test rather than an assertion about source text, which removes the urgency, and because collapsing five call sites while also adding coverage would make this diff hard to review against the original. Happy to do it as a follow-up, or here if you would rather have it in one go.
• The two you suggested filing — conductor run | head exiting 1 via on_broken_pipe, and --silent suppressing the event-log path that holds the results. Both reproduce; neither is this PR. Say the word and I will file them, or leave them to you since you have the reproductions.

Gates

535 passed in tests/test_cli. ruff check unchanged at 2 pre-existing findings; ty check src unchanged at 64; my files ruff format and ty clean.

[Jason Robert (jrob5756)]

Re-ran the per-sink mutation test against 7d5f135 rather than reading the description. The success sink is properly fixed, and that was the one that mattered.

Not ready to merge though, because the doctor case has the same problem the last round was about.

reverted sink	what notices
app.py:555 run success	AST guard and the new behavioural test
app.py:568 run terminate	AST guard only
app.py:1010 resume success	AST guard only
app.py:1016 resume terminate	AST guard only
doctor.py:102--json	AST guard only

test_doctor_data_kwarg_survives_a_cp1252_stdout never reaches doctor.py. It builds a Console and passes ensure_ascii=True from the test body. I reverted doctor.py:102 and pointed _JSON_SINK_MODULES at conductor.cli.app alone so the guard could not cover for it, and all 8 tests passed.

Your own commit message on 45611f8 puts it better than I can: a regression guard that cannot fail on the regression is worse than no guard, because it manufactures confidence. What makes this worth fixing rather than deleting is that the reasoning above it is correct. JSON.from_data really is a separate path and really is independently vulnerable. The test just demonstrates that about rich instead of about us.

A real one runs to about 12 lines. run_doctor already takes console=output_console from app.py:1415, so the patch you used for the success sink works, plus the _gather_report patch test_doctor.py relies on. I wrote it to be sure rather than to be clever: passes as-is, fails on the revert, 0.12s. Left it inline.

Two smaller ones, also inline. The comment at 127 still says a behavioural test needs a full workflow execution while the class docstring at 182 says that claim was wrong, so the file now argues with itself about whether the thing you just did was possible. And the module docstring says reverting an ensure_ascii=True argument fails a test that exercised that sink, which holds for one sink out of five.

The three untested app.py sinks are cheap if you are in there anyway. :568 and :1016 want mock_run.side_effect = WorkflowTerminated(...), and :1010 wants the resume invocation.

Everything else I checked came back fine. ty is clean on the file with no _pytest internals left. text() and _raw are pulling their weight again, and keeping newline="" and force_terminal=False is right now that the tests decode the buffer. 535 passed, ruff and format clean.

Two things I want to call out properly rather than bury. The CHANGELOG entry says out loud that doctor's table path is still broken, which is the opposite of the instinct most people have when writing one. And the doctor deferral itself was the right call: I went and looked at the threading argument before accepting it, and it holds up. #401 has a repro someone can act on.

The rest can wait, as far as I'm concerned: the shared helper, the hardcoded module list, the misattributed warning in the terminate handlers, the PEP 686 note. Say the word on the two follow-ups and I will file them, since I am sitting on the reproductions.

[Jason Robert (jrob5756)]

This never reaches doctor.py. It builds a Console and passes ensure_ascii=True itself, so what it asserts is that rich honours the keyword, which the negative control already covers from the other side.

I checked rather than assumed: reverted doctor.py:102, set _JSON_SINK_MODULES = ["conductor.cli.app"] so the guard could not cover for it, and got 8 passed.

The docstring reasoning is right, which is why this is worth fixing rather than dropping. run_doctor takes console=output_console from app.py:1415, so the patch from the test above works here too:

deftest_doctor_json_sink_survives_a_cp1252_stdout(
self, monkeypatch: pytest.MonkeyPatch
) ->None:
holder=_Cp1252Console()
report=DoctorReport(
env=EnvDiagnostic(
conductor_version="1.2.3 \u2192\u2705",
python_version="3.12.0",
platform="test",
update_checked=False,
update_available=None,
latest_version=None,
)
)
monkeypatch.setattr(doctor_mod, "_gather_report", lambda**kw: report)
withpatch("conductor.cli.app.output_console", holder.console):
result=CliRunner().invoke(app, ["doctor", "--json"])
assertresult.exit_code==0, result.outputpayload=json.loads(holder.text())
assertpayload["env"]["conductor_version"] =="1.2.3 \u2192\u2705"

Verified both directions: passes on this head, fails with doctor.py:102 reverted. 0.12s.

[Jason Robert (jrob5756)]

This is the claim the class docstring at 182 now says was wrong. Both are in the file, so it reads as though the author changed their mind halfway down.

The guard still earns its place, its job just got narrower once the behavioural tests landed:

Suggested change

	# It is deliberately a source assertion rather than a mock: driving the real
	# ``conductor run`` command to its JSON sink requires standing up a full
	# workflow execution, which belongs in an end-to-end subprocess test (tracked
	# as follow-up on #342) rather than a unit test.
	# It asserts the source rather than the behaviour, which is cheap and runs
	# anywhere. ``TestRealSinksThroughTheCli`` covers the behaviour, so this
	# guard's remaining job is the narrower one: catching a *new* sink added
	# without the keyword, which is what a source assertion is actually good at.

[Jason Robert (jrob5756)]

True of the run success sink. Revert any of the other four and only the AST guard notices, so this is still promising a bit more than the file delivers.

Worth getting exactly right given the paragraph directly below makes a point of the file having overclaimed once already. If the doctor case gets its own behavioural test, this becomes true of two sinks and the data= mention above stays honest:

Suggested change

	Patchingonlytheconsole, neverthecodeundertest, meansrevertingan
	``ensure_ascii=True``argumentfailsatestthatexercisedthatsink.
	Patchingonlytheconsole, neverthecodeundertest, meansrevertingthe
	``ensure_ascii=True``ona*covered*sinkfailsatestthatexercisedit. The
	run-successand``doctor``sinksarecoveredthatway; theterminateandresume
	sinksstillrestontheASTguardalone.

[Frank Li (franklixuefei)]

Thanks -- the doctor finding was right, and it was the same defect as last round, which is the part worth sitting with. My own commit message said a guard that cannot fail on the regression manufactures confidence, and I then shipped exactly that one file down.

Fixed in 70b2093.

The doctor sink

run_doctor takes console=output_console from app.py:1415, so the patch the success sink uses works here too. Patch only the console, patch gather, drive the real function:

monkeypatch.setattr("conductor.cli.doctor.gather", _fake_gather)
run_doctor(section="env", ..., as_json=True, console=holder.console, err_console=holder.console)
payload=json.loads(holder.text())

I ran your check rather than assuming: reverted doctor.py:102and set _JSON_SINK_MODULES = ["conductor.cli.app"] so the guard could not cover for it. 1 failed, 7 passed -- and the failure is the new test.

The other three sinks

Done, since I was in there. :568 and :1016 take side_effect = WorkflowTerminated(...), :1010 the resume invocation. resume takes --from, not --checkpoint, which cost me a few minutes.

Full per-sink matrix, run with _JSON_SINK_MODULES = [] so only behavioural tests are in play:

reverted sink	before	now
app.py:555 run success	caught	caught
app.py:568 run terminate	AST guard only	caught
app.py:1010 resume success	AST guard only	caught
app.py:1016 resume terminate	AST guard only	caught
doctor.py:102--json	AST guard only	caught

Each revert fails exactly one test, and it is the one that exercises that sink.

The docs

All three were fair.

The file argued with itself -- the comment at 127 claimed a behavioural test needed a full workflow execution while the class docstring said that claim was wrong. Took your wording for the guard comment; its job really is the narrower one now. I also named its actual blind spot, the hardcoded module list, since checkpoint.py is the plausible next sink and the guard would not notice.

The module docstring said reverting an ensure_ascii=True fails a test that exercised that sink. That held for one sink of five. Rather than hedge the sentence I made it true, and it now says so with the condition attached: with the source guard disabled, reverting any one of the five fails exactly one behavioural test.

Class docstring said "the AST guard below" -- it is above.

Follow-ups

Yes please, if you are still sitting on the reproductions: the shared cp1252 helper, the hardcoded module list, the misattributed warning in the terminate handlers, and the PEP 686 note. I would rather they were filed by whoever has the repro than described by me second-hand.

535 passed on tests/test_cli, ruff and format clean. Note my local ruff was ahead of the lock (0.16.2 vs 0.14.14) and reported two UP042s that CI does not see; I pinned to the locked versions before trusting any of the above.

[Jason Robert (jrob5756)]

LGTM. Approved!