fix(tray): tokenize NSSM AppParameters with CommandLineToArgvW semantics (BACKLOG #1565)

[wshallwshall]

Closes the tokenizer half of BACKLOG #1565. Two commits: the fix, then a /simplify pass over it.

What was wrong

messagefoundry/tray/config.py tokenized the NSSM AppParameters string with str.split(). That keeps the quote characters AND still splits inside them, so --service-config "C:\Program Files\MF\x.toml" became "C:\Program and Files\MF\x.toml" — two paths that cannot exist. The engine's settings then read as absent, and the tray fell back to its default for the served scheme.

Both of the row's corrections hold against the tree, and both are now pinned:

• a quoted path with no space broke identically, so the defect is the quoting rather than the space
• quoted --host and --port values failed _VALID_HOST and int(), so the discovered URL was wrong by more than its scheme

The fix

One new private helper, _split_command_line, and one line changed in _iter_options. _iter_options is defined once and called twice, both inside this file, so that is the whole radius. The = split is unchanged and now runs on an already-dequoted token, which makes the quoted equals form work for free.

Why shlex was rejected, in both modes

The row offered shlex(posix=False) or CommandLineToArgvW semantics as alternatives. They are not alternatives — neither shlex mode works:

mode	on --service-config="C:\Program Files\MF\x.toml"	on the unquoted C:\data\x.toml
posix=False	keeps the quotes, still splits at the space	survives
posix=True	dequotes correctly	destroyed: C:datax.toml

posix=True eats the backslashes, which breaks the unquoted Windows path that works today. So the Win32 rules are hand-rolled in pure Python. A ctypes call into shell32 was also rejected for the shipped path: the tray is stdlib-only and this module's pure core must stay unit-testable on any OS, so a ctypes tokenizer would red the Linux leg.

Correctness is measured, not argued

The tokenizer is pinned against the real shell32.CommandLineToArgvW through ctypes, in a test guarded by skipif(sys.platform != "win32"). ctypes appears only there, never in the shipped path.

• 24 curated cases plus 4000 seeded fuzz cases, zero mismatches. Locally I ran the same differential at 40,000 fuzz cases, also zero, and re-ran it unchanged after the /simplify restructure.
• The oracle caught a real error in my first implementation: I had "" inside a quoted run staying quoted (the MSVCRT rule). CommandLineToArgvW ends the run there. 156 of 40,000 fuzz cases diverged on that one point and nothing else.
• It also caught a wrong expectation I hand-wrote: I expected "a""b" c to give ['a"b', 'c']. The real function gives ['a"b c'], because the later quote reopens a run. The code was right and my reading was wrong.

The oracle test now runs over the unit test's own case table, so a case added to one is checked by both.

engine_serves_https is deliberately untouched

The row's heading reads like two defects. It is one, and the row body says so: "The scheme flip is the consequence, not the bug." Misparsing loses the config file; no file means no settings; no settings imply https.

That last step is correct and must stay. Since ADR 0172 the engine always mints, so absent or unreadable settings must answer True — the old cert-path predicate is BACKLOG #1126, and it rendered a running engine as WEDGED on the commonest posture. Three rows of test_engine_serves_https pin it ({"api": "not-a-table"}, {}, None). Changing it would red those three and reopen a closed defect.

The row's two acceptance bullets for an operator certificate and a declared upstream terminator were already met by that same test, including the ordering case where a cert set alongside a declared proxy still returns True. Not rebuilt.

Which parser this mirrors, and why that is a real question

NSSM hands AppParameters to CreateProcess, so the engine's own sys.argv comes from the C runtime, not from shell32. The MSVCRT rules differ from CommandLineToArgvW on exactly one point: a doubled quote inside a run stays quoted there and ends the run here.

I measured it against a real child process on twelve command lines. Every line carrying a path, a host or a port agreed. The only three that diverged were built purely of quotes ("a""b" c, a """ b, """""), and none of those can reach anything this file acts on: a quote is not a legal Windows filename character, _VALID_HOST rejects it, and --port goes through int(). CommandLineToArgvW stays the model, and the docstring now records the divergence so the next reader does not re-derive it.

What this does NOT cover

Named deliberately, so none of it reads as an oversight:

1. Apostrophe-quoted values stay "broken", and that is correct. 'C:\x' is not a Windows quoting form; the real CommandLineToArgvW keeps the apostrophes, so the path genuinely is named with them. Pinned as a test row because it looks like a gap and is not one.
2. --config and --db quoted values remain unparsed. Harmless: neither flag is in wanted, so their fragments were skipped before and their dequoted values are skipped now.
3. A trailing-backslash path like "C:\MF\\" resolves to a directory. _read_toml then fails and engine_serves_https(None) answers True. Fail-soft, unchanged, intended.
4. The stock install is not the posture this fixes. scripts/service/install-service.ps1:426 writes no --service-config at all. Its quoted --config/--db values did fragment under the old split, but harmlessly, and --host/--port sat outside the quotes. I ran the shipped line through both tokenizers with a spaced install path: ('127.0.0.1', 8765) and no --service-config either way. So a stock install parses identically before and after this change, and the defect bites a hand-edited AppParameters.

Severity

No live exposure — zero deployments (CLAUDE.md section 0). Conditionally: a deploying site that had declared tls_terminated_upstream and hand-edited a quoted --service-config path would have had its tray probe https against an engine deliberately speaking plaintext to its proxy, and render a running engine as WEDGED. test_load_config_finds_a_quoted_service_config_path_with_a_space is that case end to end.

Checks

Run in this worktree's .venv, foreground:

• ruff check messagefoundry tests — passed
• ruff format --check — passed
• mypy messagefoundry (strict) — passed, 275 source files
• pytest tests/test_tray_config.py — 86 passed
• every tray-adjacent module plus test_cert_cli.py and test_service_control.py — 281 passed, 1 skipped
• the win32 oracle test was confirmed to RUN rather than skip (-k oracle -v, 0.02s call), since a skip reads as a pass

Local runs report INCOMPLETE RUN -- coverage was NOT collected because the vault extra is absent from this interpreter. Expected, and named here rather than hidden.

Not run: the full suite. The Windows leg is a 29-36 minute job and would not finish inside one Builder turn. The change is confined to one private helper whose only consumers are the two functions above it in the same file (verified by grep across messagefoundry/, tests/, harness/, ide/, scripts/).

Legs to read after my process exits: the Windows leg, because test_split_command_line_matches_the_win32_oracle runs nowhere else — the Ubuntu leg skips it and pays nothing.

Proposed ledger banner text for #1565

Fixed 2026-09-16 -- PR pending. The tray now tokenizes AppParameters with CommandLineToArgvW semantics (_split_command_line in messagefoundry/tray/config.py), pinned against the real shell32 function by a differential test on the Windows leg. Quoted paths with and without spaces, the quoted equals form, quoted --host/--port, malformed quoting and relative paths all parse. engine_serves_https was deliberately left alone: its absent-settings-imply-https answer is BACKLOG #1126 and ADR 0172, not a second defect, and the row's certificate and upstream-termination acceptance bullets were already pinned by test_engine_serves_https.

Not done

docs/BACKLOG.md is untouched — it is the 23-line public stub, and the ledger is vault-only.

[wshallwshall]

Lander review

The finding is better than the fix, and it is the one I would carry out of this PR. The brief said a hand-rolled tokenizer already matched shell32.CommandLineToArgvW on 14 of 14 command lines. The Builder re-ran it larger and diverged on 2 of 26 curated and 156 of 40,000 fuzz cases — the MSVCRT rule keeps "" inside a quoted run quoted, and CommandLineToArgvW ends the run there.

The original measurement was not wrong about what it ran. It was wrong about what that licensed. A 14-case corpus is too small to contain the discriminating shape, and a clean result from an underpowered instrument is indistinguishable from a clean result from a sufficient one. That is the same family as a bounded query's false zero, one level up — and it is the third distinct instance of that family I have seen today, after a --limit-shaped absence and an in-progress job census read as complete.

An executable oracle caught what review could not. The Builder's own hand-written expectation predicted "a""b" c yields ['a"b', 'c']; the real function yields ['a"b c'], because the later quote reopens the run. Code right, reading wrong. No amount of careful reading finds that — only running the real function does.

Verified rather than read

engine_serves_https is genuinely untouched. It appears once in the diff and only as a context line in an import — no + or - on it. Your "verified zero" holds.

Pinned against the real function, not a model of it. @pytest.mark.skipif(sys.platform != "win32", reason="shell32.CommandLineToArgvW is Windows-only") with a differential described as "the rules, not a reading of them". Windows-only by necessity, which is why the Windows leg carries the whole oracle and Ubuntu skips it.

The cross-parser divergence is the right kind of recorded non-finding

The tray models CommandLineToArgvW, but the process receiving AppParameters is the Python engine, whose sys.argv comes from the C runtime — and the two disagree on doubled quotes. Unreachable in practice, and the reasons are checkable rather than asserted: a quote is not a legal Windows filename character, _VALID_HOST rejects it, --port goes through int(). Across twelve command lines only the three built purely of quotes diverged.

Putting that in the docstring rather than a PR comment is correct — the next reader would otherwise re-derive it, and the PR body will not be in front of them.

Your half-wrong bullet, handled the right way

install-service.ps1:426 does write quoted --config and --db values that fragmented under the old split. Harmless because neither flag is in wanted and --host/--port sit outside the quotes — and the Builder ran the shipped line through both tokenizers with a spaced install path rather than reasoning it, getting ('127.0.0.1', 8765) either way.

Legs I will read after the fact

The Windows leg carries the oracle entirely. Full suite not run, 29-36 minutes, and the PR body says so.

Verdict: merge.

[github-actions]

CI failed while this pull request was in the merge queue, so the queue ejected it.

Its own head can still be green: the queue revalidates the merge, and the path gates that skip on a pull request run there. Read the run before retrying.

https://github.com/MEFORORG/MessageFoundry/actions/runs/35137321302

[wshallwshall]

LANDER INSPECTION -- labelled self-review, not a peer review

Posted under the korus LANDER.md line 364 obligation: with required_approving_review_count: 0
I am the last reader. I did not author this change. This PR already carried a verdict; that
does not discharge my own read, so this is mine.

The defect and the two rejected alternatives, verified by execution

I ran all three claims rather than reading them:

str.split()        --service-config "C:\Program Files\MF\x.toml"
                   -> ['--service-config', '"C:\Program', 'Files\MF\x.toml"']

shlex posix=True   C:\data\x.toml   -> ['C:datax.toml']
shlex posix=False  "C:\Program Files\MF\x.toml" -> ['"C:\Program Files\MF\x.toml"']

The defect is exactly as described. str.split() both keeps the quote characters and splits
inside them, so the setting reads as absent and the tray silently falls back to its default served
scheme. A quoted path with no space breaks identically -- the quoting is the defect, not the
space -- which the docstring says and which the third line above confirms.

Both shlex modes are genuinely unusable. posix=True turns the unquoted C:\data\x.toml that
works today into C:datax.toml -- a working configuration broken by the fix. posix=False keeps
the quote characters, so the path never resolves.

One imprecision in the docstring, which does not change the conclusion

posix=False keeps the quote characters and still splits inside them

Measured, posix=False keeps the quotes but does not split inside them -- it returns one token.
The "splits inside them" half describes str.split(), the behaviour being replaced, not shlex.

The rejection still stands on the first half alone: retained quote characters break the path. Worth
correcting if the file is touched again, because the sentence as written would mislead the next
person evaluating shlex for a different caller.

The part that shows real care

Which parser to mirror is a real question, and the answer was measured. NSSM hands
AppParameters to CreateProcess, so the engine's own sys.argv comes from the C
runtime, and the MSVCRT rules differ from these on exactly one point [...] It does not reach
anything this file acts on -- a quote is not a legal Windows filename

Identifying an ambiguity between two real parsers, naming the single point of divergence, and then
showing that point is unreachable for this caller. That is the difference between picking a spec and
justifying one.

The reason for not reaching into shell32 via ctypes is equally concrete: this module's core does no
OS-specific I/O and is unit-testable on any OS, and a ctypes call would end that.

Verdict

Verdict: merge.

[github-actions]

CI failed while this pull request was in the merge queue, so the queue ejected it.

Its own head can still be green: the queue revalidates the merge, and the path gates that skip on a pull request run there. Read the run before retrying.

https://github.com/MEFORORG/MessageFoundry/actions/runs/35167160562