fix(corepoint): stop emitting a live field-write stub, and harden the literal and comment renderers (BACKLOG #1681, #1683)

[wshallwshall]

Closes BACKLOG #1681 and #1683. One PR because both edit messagefoundry/corepoint_import.py and tests/test_corepoint_import.py, and #1681 deletes the two _lit(step.stub_path) call sites that #1683 would otherwise have hardened.

#1681 -- the live field-write stub is withdrawn from the two layers that still emitted it

_generate_steps emitted msg.set(<target>, msg.field(<target>) or "") for every UnmappedAction carrying a stub_path. _map_action (the superseded JSON layer) and _map_statement (the fallback for an export whose @Data carries no span markup) both still set one.

That line was never the inert passthrough its own comment claimed. Message.set raises KeyError on an absent segment, and on a present segment with an absent field it materialises the field and its empty components on the wire. _decline, on the validated role-parsed path, had already withdrawn it on exactly those grounds and left the reasoning in a comment. This finishes the job.

UnmappedAction.stub_path is deleted, not left unused, so there is no field for a future caller to re-populate. The recovered target is not lost: it rides into the marker text as ; intended target <path>, which is what _decline already did.

#1683 -- repr() was rejected, and the evidence is _collect_sends

The row prescribed rendering strings with repr(). That is fatal, not merely disruptive:

• _collect_sends parses _lit's output back as JSON. It recovers a MsgSend destination with json.loads(step.args[0]), where args comes from _role_send_args / _send_args, both of which render through _lit. repr produces a single-quoted Python string that json.loads rejects, so the shipped acme_adt_package.xml fixture dies on an uncaught JSONDecodeError before any module is written.
• The quote style is a contract, not a style choice. 39 assertions in tests/test_corepoint_import.py pin _lit's double-quoted output, and ruff's format gate wants double quotes in the generated module too. json.dumps gives double, repr gives single.

So _lit stays json.dumps, gains ensure_ascii=False, and gains a refusal pass. The ensure_ascii=False half mirrors messagefoundry/lens.py::_render_literal (read, not edited) -- the default ASCII escaping turns an astral code point into a \uXXXX surrogate pair that Python re-reads as two lone surrogates.

The refusal is narrower and wider than the row asked

The row prescribed "refuse non-string scalars". That is too broad: _lit is legitimately handed the ItemCodeLookup table dict and the ItemSplit destinations list, and a blanket refusal breaks working input. test_a_nested_container_of_strings_still_renders pins that they keep working.

It is also too narrow: the genuinely unguarded values were raw["default"] and the table's own values. {"table": {"M": null}} renders {"M": null}, which is a NameError the moment the generated module is imported.

_assert_renderable therefore walks containers and refuses exactly what JSON renders and Python cannot read back:

Value	JSON renders	Python reads it as
null / true / false	null / true / false	undefined names
NaN / Infinity / -Infinity	the bare token	undefined names
unpaired surrogate	the raw code point	parses, then UnicodeEncodeError at file write
non-string dict key	a silently coerced string	a different value than the export carried

Ints, finite floats, lists and dicts of those all still render. The error names the position (export value['M'] is ...) so a 200-entry lookup table says which entry is bad.

The surrogate check is text.encode("utf-8") in a try, not a per-character code-point scan: it is the same operation import_corepoint performs later, so there is no second definition of "encodable" to drift out of step with it, and it is 5x-77x faster by length.

The injection hole #1681's own prescription would have opened, and how it is closed

This is the finding that changed the shape of the fix. #1681 prescribed moving the recovered target into a # comment. On the JSON layer, _map_action's target is raw.get("target") or raw.get("destination") or raw.get("source") -- arbitrary, unvalidated JSON that has been through no grammar. Today it is contained only by _lit, and a _lit literal contains a newline by escaping it, while a comment does not: the line simply ends and whatever follows is a statement.

So the row's own fix, applied literally, converts a contained value into a top-level code injection. An export with {"class": "ItemMystery", "target": "MSH-6\nimport os\nos.system(...)"} yields a module that still compiles and imports os. Measured -- see the control runs below.

Both source_class and the recovered target now go through the hardened _comment_text at the render site (_generate_steps), which is the single escape boundary for the comment path. They stay raw at construction deliberately: a comment needs a different escape from a literal, the renderer is the one place that knows a value is about to become a comment, and there are more places that build an UnmappedAction than render one. Escaping at both layers would leave a reader unable to tell which is the contract -- and the natural "cleanup" would be to delete the load-bearing one.

_comment_text also now deletes non-whitespace control characters. NUL is the member that matters: Python refuses to compile a source string containing one, so a single NUL in an export's class name turned the whole generated module into a file that cannot be imported. The alphabet comes from messagefoundry/controlchars.py::strip_control_chars rather than being spelled out an eighth time -- that module exists precisely to stop the copying.

Second render site closed: _count_steps collects source_class into unmapped_classes, which the CLI prints in its import summary. Because the taint is neutralised at the render site rather than at construction, that sink inherited nothing. A class named "Foo\n IB_X.py (400 mapped)" would forge a line in the count-and-log record a migrator trusts. Same escape, different sink.

The four tests that pinned the stub

Test	Was	Now
test_unmapped_action_is_stubbed_not_dropped (renamed to ..._is_marked_not_dropped)	stub_path == "OBX-5", 'msg.set("OBX-5", msg.field("OBX-5") or "")' in src	"intended target OBX-5" in the detail and in the source; _assert_no_live_stub
test_unmapped_without_target_emits_marker_only	step.stub_path is None, "msg.set(" not in src	"intended target" not in step.detail; _assert_no_live_stub
test_unmapped_verb_emits_a_todo_marker_and_is_counted	'msg.set("OBX-5", ...)' in src	"intended target OBX-5" in src, "msg.field(" not in src
test_a_path_that_does_not_resolve_is_never_guessed	'msg.set("PID-3.1"' in src, 'msg.set("NK1-2.1"' in src	"intended target PID-3.1"/"...NK1-2.1" in src; _assert_no_live_stub

The rename is carried into ADR 0086's AC-2 pointer -- those were the only two places the old name appeared.

Why "msg.field(" not in body is the stronger assertion. It pins the withdrawn stub's read, which no marker or hint text has any reason to mention, so it still fails on a passthrough rewritten to write through something other than Message.set -- which is the shape a partial revert would take. "msg.set(" alone would pass such a revert. Both are asserted, in one helper (_assert_no_live_stub) rather than four copy-pasted pairs, matching the already-hardened test_a_declined_statement_emits_no_live_stub.

New tests, each verified to fail with its own fix reverted

Two control runs, hardening reverted one mechanism at a time, then restored:

Test	Failure mode without the fix
test_an_unmapped_actions_recovered_target_cannot_escape_its_comment	import os becomes a real top-level statement; the module still ast.parses
test_a_nul_in_an_action_class_cannot_make_the_module_uncompilable	\x00 reaches the source; compile() raises ValueError
test_a_json_scalar_python_cannot_read_is_refused_not_rendered (3 cases)	no raise -- {"M": null} / default=null / {"M": true} written into the module
test_an_unpaired_surrogate_is_refused_before_it_reaches_the_file	no raise; the surrogate reaches write_text(..., encoding="utf-8")
test_a_nested_container_of_strings_still_renders	passes in both arms by design -- it is the "did I break working input" control, not a regression net

The parametrized case asserts the position in the message (export value['M'] is), not just the token, so the three cases can actually tell each other apart.

ADR 0086

Amendment (c'') withdraws the passthrough stub. The original (c) paragraph is kept as written rather than edited, because the stub shipped and a reader needs to recognise the line in a module generated before today; the amendment names the two sentences that no longer describe the code. AC-2 and AC-5 are updated, and docs/adr/README.md's index row -- which still asserted the stub as the general rule, and is what a reader hits before the ADR body -- is corrected.

tests/test_lens_parse.py has a two-word docstring/comment correction ("TODO stub" -> "TODO marker"). The kinds.count("code") == 1 assertion is unchanged and still passes: the lens classifies the bare marker comment as one code row, so AC-4 is untouched. PR 1170 is merged into this base, so lens.py itself is not edited here.

Checks run, in this worktree's venv, all foreground

• ruff format --check . -- 1305 files already formatted
• ruff check . -- all checks passed
• mypy messagefoundry (strict) -- no issues in 275 source files
• pytest tests/test_corepoint_import.py tests/test_lens_parse.py -- 105 passed
• full suite pytest -q with QT_QPA_PLATFORM=offscreen -- 3280 passed, 116 skipped, 8 xfailed
• /simplify -- four review agents; fixes applied below

One pre-existing local failure, unrelated and environmental. tests/test_ci_retry_native_crash.py fails 6 of 10 on this box because the test invokes C:\windows\system32\bash.EXE (WSL bash), which cannot resolve the Windows path C:/Users/.../scripts/ci/retry-native-crash.sh. It fails identically against the committed tree and touches none of the five files this PR changes. It should pass on a Linux runner.

Legs a Builder never sees -- please read these after the run: windows-service-smoke (NSSM), and any leg needing the vault extra, which is absent from this interpreter (the suite says so itself at the end of every run, so the 3280 above does not cover vault-gated modules).

From the /simplify pass

Applied: the C0+DEL alphabet now reuses controlchars.strip_control_chars instead of a private table; _comment_safe folded back into _comment_text (the split was a default argument in disguise); _assert_renderable's breadcrumb simplified to a plain string and given a test; the surrogate scan replaced by encode("utf-8"); the construction-time escape in _map_action removed in favour of the single render-site boundary; _one_action_export folded together with _named_inbound_export and the two inline copies of the same JSON skeleton retired.

Skipped, deliberately: 18 renders of Control.source_verb into comments are still unescaped. They are safe by grammar today -- every construction site is either an XML local Name (which cannot contain whitespace or NUL; NUL is not a legal XML character, so expat rejects it first) or a string matched by _VERB = ^[A-Za-z][A-Za-z0-9_]*$. Two of the 18 are trailing comments on live statements, which is the sharp shape. Nothing is measured broken, so fixing them is out of scope here -- but widening _VERB would silently arm all 18, and that is recorded here so the next person knows.

Also skipped: messagefoundry/__main__.py still says "TODO stubs" in its import subparser help and its summary line. Open PRs 1191, 1192 and 1194 hold that file, so this PR stays out of it. docs/backlog-proposals/fable-packet10b-rootrest.md still describes the stub as unfixed; it is a dated packet snapshot, so it is left as the historical record.

#1682 is NOT in this PR

#1682 (the importer recurses once per sibling branch marker) also edits corepoint_import.py and was not measured. It is untouched here and should be expected to rebase onto this result -- _generate_steps, _map_action, _map_statement and _decline all moved.

Proposed ledger banner text (vault-only; docs/BACKLOG.md not edited)

#1681 -- SHIPPED 2026-09-16, PR 1197. UnmappedAction.stub_path deleted; _map_action and _map_statement pass no stub target and carry the recovered field in the marker text as _decline does; the msg.set emission is gone from _generate_steps. Four stub-pinning tests rewritten to "msg.field(" not in body. ADR 0086 amendment (c'') + AC-2 + the ADR index row. The row's own prescription (move the target into a comment) opens a code-injection hole on the JSON layer, where the target is arbitrary unvalidated JSON; closed by routing it through the hardened _comment_text at the render site, with a fixture.

#1683 -- SHIPPED 2026-09-16, PR 1197. The row's repr() prescription is fatal, not disruptive: _collect_sends does json.loads on _lit output and the shipped XML fixture dies on JSONDecodeError; repr also breaks the double-quoted output 39 assertions treat as a contract. Used json.dumps(..., ensure_ascii=False) plus a refusal pass instead. The row's "refuse non-string scalars" is both too broad and too narrow -- the code_lookup table dict and split_field destinations list are legitimate _lit input; the genuinely unguarded values were raw["default"] and the table's own values. _comment_text now strips non-whitespace controls via controlchars.strip_control_chars. Five fixtures, each verified to fail with its fix reverted.

[wshallwshall]

Lander review

The find is the sink the brief did not name, and it is a count-and-log defect rather than a cosmetic one. _count_steps feeds unmapped_classes into the CLI import summary, so a Corepoint class named "Foo\n IB_X.py (400 mapped)" forges a line in the record. Neutralising at the render site left that sink inheriting nothing. CLAUDE.md section 1 makes the count-and-log record load-bearing, so a forged line in it is the right severity.

Escaping at ONE boundary, proved rather than asserted. Sanitising at construction and at render looked safer, and the measurement is what settled it: doing both neutered the construction-time escape and produced a byte-identical marker line for the hostile payload. Nothing pinned that, so a future reader would have deleted the load-bearing half believing it redundant. Two escapes that produce identical output are indistinguishable until one is removed.

It uses the shared alphabet. strip_control_chars from messagefoundry/controlchars.py, not a private table — that module's docstring records seven copies consolidated for exactly this reason, and an eighth would have been the same defect in a new place.

flat = strip_control_chars(" ".join(text.split()))

Whitespace collapse then C0+DEL strip. Both halves needed: split() alone leaves NUL, strip_control_chars alone leaves the line structure.

Scope confirmed: zero lens.py hunks in the diff. The two-word edit in tests/test_lens_parse.py was only safe because PR 1170 had already merged, which is worth noting since the brief said 1170 was open.

The residual is recorded at its true size. 18 renders of Control.source_verb into comments are unescaped and safe by grammar — every source is an XML local Name or a _VERB regex match, neither able to carry a newline or NUL — with two of them trailing comments on live statements. "Widening _VERB silently arms all 18" is the sentence that makes this a recorded hazard rather than a shrug; it names the single edit that would convert safe-by-accident into exploitable.

All five new tests verified to fail with their own fix reverted, in two control runs. The test_ci_retry_native_crash.py failure is environmental and fails identically against the committed tree.

Verdict: merge.