refactor(printing): put every Windows printing call behind a tested seam, changing nothing that reaches Windows #42

Merged
james.maes merged 13 commits from refactor/2.0-native-seams-printing-20261110 into release/2.0 2026-11-11 02:08:16 +00:00
Owner

Plan step 7 of the multi-server build: every Windows printing call (spooler, GDI, printer settings, PDF rendering, spooler job queue, the helper's parent watch) moves behind a small adapter, and every decision stays in code that tests cover with hand-written fakes.

The rule for this change: nothing that reaches Windows changes. Which native calls are made, their arguments and order, the abort and close on each failure path, and exception types and messages are the same as before. The reviewer derived the call sequences from the old and the new code independently and found them identical.

What moves

  • WindowsPrinters (discovery, ZPL, PDF, media) calls ISpooler, IPrinterSettings, IGdiPrinter and IPdfRenderer.
  • Spool cancellation rules are separate from the spooler job queue.
  • The print helper takes its parent watch, spooler and streams as arguments.
  • Six adapters are excluded from coverage, each with a written reason that lists the arguments it fixes.

One behaviour change, declared

  • The print helper bounds its request at 1 MiB while it is read (it used to read the whole line first). The largest legitimate request is about 14 KB.

Verification

  • Head 5d2a030 on release/2.0 at 077148f.
  • build/Verify.ps1 passed on that head: 773 Core, 1,521 Windows runtime and command line, 352 presentation tests, none failed; Core, presentation and command line at 100%; Windows 87.47% (3,835 of 4,384 lines).
  • Windows minimum in build/minimum-coverage.json raised from 80 to 86.

Review (independent reviewer, two passes)

  • 2062c24: no blockers; two should-fix about test evidence (fakes left the Windows last error sticky; an untrue exclusion reason), nits.
  • 5d2a030, the head merged: Verdict on 5d2a030: 0 blockers, 0 should-fix, 2 nits. Product code under src changed in the fix round only in comments, exclusion reasons and blank lines. The reviewer's own late-error-read mutations now fail tests.
  • Replays onto 6f0237d and 077148f: patches identical except the coverage-minimum hunks and one summary comment in the worker fixture, merged by hand and checked by the reviewer.

Not proven here: real hardware

  • The real adapters have never run against real Windows printing in a test. Runtime marshaling, real driver answers, output geometry and the spooler's own error codes can only be shown on printers.
  • The owner agreed two checks on real printers: an early one after this merges (without installing, by driving the built helper), and a final one on the release candidate. "Ready to merge to main" waits for the final check.

Left open, to be recorded in the audit

  • Older behaviour, unchanged here: every job runs a full printer discovery (being fixed separately at the owner's request); the helper's output is read whole before its size is checked; the bitmap bound scales with the display scale; a PDF with no pages is spooled empty and recorded as spooled; a page under one device pixel fails after the document started with an error that is not its own.
  • Nits: the fixture's fake OpenPrinter reports the marker error for an unknown queue; one test property lacks documentation; allocation before try in WindowsPrinterSettings; an unchecked cast in WindowsSpoolerJobQueue.

🤖 Generated with Claude Code

Plan step 7 of the multi-server build: every Windows printing call (spooler, GDI, printer settings, PDF rendering, spooler job queue, the helper's parent watch) moves behind a small adapter, and every decision stays in code that tests cover with hand-written fakes. **The rule for this change: nothing that reaches Windows changes.** Which native calls are made, their arguments and order, the abort and close on each failure path, and exception types and messages are the same as before. The reviewer derived the call sequences from the old and the new code independently and found them identical. **What moves** - `WindowsPrinters` (discovery, ZPL, PDF, media) calls `ISpooler`, `IPrinterSettings`, `IGdiPrinter` and `IPdfRenderer`. - Spool cancellation rules are separate from the spooler job queue. - The print helper takes its parent watch, spooler and streams as arguments. - Six adapters are excluded from coverage, each with a written reason that lists the arguments it fixes. **One behaviour change, declared** - The print helper bounds its request at 1 MiB while it is read (it used to read the whole line first). The largest legitimate request is about 14 KB. **Verification** - Head `5d2a030` on `release/2.0` at `077148f`. - `build/Verify.ps1` passed on that head: 773 Core, 1,521 Windows runtime and command line, 352 presentation tests, none failed; Core, presentation and command line at 100%; Windows 87.47% (3,835 of 4,384 lines). - Windows minimum in `build/minimum-coverage.json` raised from 80 to 86. **Review (independent reviewer, two passes)** - `2062c24`: no blockers; two should-fix about test evidence (fakes left the Windows last error sticky; an untrue exclusion reason), nits. - `5d2a030`, the head merged: `Verdict on 5d2a030: 0 blockers, 0 should-fix, 2 nits.` Product code under `src` changed in the fix round only in comments, exclusion reasons and blank lines. The reviewer's own late-error-read mutations now fail tests. - Replays onto `6f0237d` and `077148f`: patches identical except the coverage-minimum hunks and one summary comment in the worker fixture, merged by hand and checked by the reviewer. **Not proven here: real hardware** - The real adapters have never run against real Windows printing in a test. Runtime marshaling, real driver answers, output geometry and the spooler's own error codes can only be shown on printers. - The owner agreed two checks on real printers: an early one after this merges (without installing, by driving the built helper), and a final one on the release candidate. "Ready to merge to main" waits for the final check. **Left open, to be recorded in the audit** - Older behaviour, unchanged here: every job runs a full printer discovery (being fixed separately at the owner's request); the helper's output is read whole before its size is checked; the bitmap bound scales with the display scale; a PDF with no pages is spooled empty and recorded as spooled; a page under one device pixel fails after the document started with an error that is not its own. - Nits: the fixture's fake `OpenPrinter` reports the marker error for an unknown queue; one test property lacks documentation; allocation before `try` in `WindowsPrinterSettings`; an unchecked cast in `WindowsSpoolerJobQueue`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
james.maes added 13 commits 2026-11-11 02:07:56 +00:00
WindowsSpoolerJobs mixed the rule that protects other people's print
jobs with the four spooler calls that carry it out, so none of its 24
measured lines could run without a real print queue and a job to delete.

The calls now sit behind ISpoolerJobs, one member per Windows call:
open a queue, ask the size of a job's details, read the job's document
name, send the job a command, close the queue. WindowsSpoolerJobQueue is
the real one and is excluded from coverage with its reason. The rules
stay in WindowsSpoolerJobs and are unchanged: the receipt must be a
plain unsigned number, a job that left the queue (error 87 or 1168) is
reported as already absent, the answer is bounded at 1 MiB, the document
name must equal "QQQ <job id>" exactly or cancellation is refused as an
ownership mismatch, and the queue is closed on every path. Messages,
exception types, the delete command and the order of calls are the same.
A failed call still leaves its error as the thread's last error, which
the rules read where they read it before.

Tests run the rules against a spooler that exists only in memory and
assert the calls sent and their order: the match, twelve near-miss
document names, a job that is gone, an unreadable job, a queue that
cannot be opened, a failed read, a refused delete, the bound and an
invalid receipt.

Windows line coverage measured 72.24 percent; the minimum rises from 69
to 71.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Reading a queue's machine defaults and preparing a job's settings mixed
the rules (every bound, what counts as usable paper and resolution,
whether a driver's answer is accepted) with the Windows calls, so 107
of the 188 measured lines of WindowsPrinters.Media.cs and .PdfMedia.cs
could only run against an installed queue and its driver.

Three seams now carry the calls, one member per Windows call, each with
a real class that is excluded from coverage with its reason:
IPrinterSettings (GetPrinter, DeviceCapabilities, DocumentProperties),
ISpooler (the winspool.drv queue and document calls) and IGdiPrinter
(the gdi32.dll device context and page calls). The spooler and GDI
seams are introduced whole here; this change moves the settings reads
onto them, and the next one moves discovery and the two document paths.

The rules stay in WindowsPrinters and are unchanged. The level 8 answer
and the driver's paper lists are now copied out by the adapter and read
as bytes, so the pointer and length checks that guarded native memory
guard the copy instead, with the same bounds. Calls, their arguments
and their order are the same; so are the messages and exception types.
A failed call still leaves its error as the thread's last error.

Tests run the rules against a Windows that exists only in memory and
assert the calls sent, in order, and the settings bytes handed over:
the level 8 read and its bounds, the pointer and length checks, the
paper lists and every way they can be untrustworthy, explicit and
context resolution, the media recheck, the driver merge and its
refusals, and job paper.

Windows line coverage measured 75.07 percent; the minimum rises from 71
to 74.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
WindowsPrinters.cs and WindowsPrinters.Files.cs mixed what the adapter
decides (which queues may print unattended, the queue identifier, the
formats offered, the checks on a staged file, the copy bounds, the copy
and page loops, where a page lands on the paper) with the calls that
list queues and send a document, so 126 of their 141 measured lines
could only run by listing this computer's queues or printing on one.

Discovery, the RAW ZPL document and the PDF document now reach Windows
only through ISpooler and IGdiPrinter, and the PDF renderer through a
new seam, IPdfRenderer, with WindowsPdfRenderer as the real one,
excluded from coverage with its reason. The private Native class is
gone; its declarations live unchanged in the three adapters.

Nothing about what reaches the spooler changes: the document name
"QQQ <job id>", the RAW data type, the 64 KiB pieces and the retry of a
partial write, one whole file per copy, the media recheck between
opening the queue and starting the document, the device context built
from the job's own settings, the media check before StartDoc, the page
geometry, stretch mode 4 and a plain copy of the whole bitmap, and on
every failure the same abort and the same release. Messages and
exception types are the same.

Tests run the rules against a Windows and a renderer that exist only in
memory, with staged files in temporary folders, and assert the calls
sent, in order, with their arguments: listing and classifying queues,
a failing spooler, the checks before anything is spooled, RAW copies,
partial and impossible writes, PDF pages at actual size and fitted,
the bound on the rendered bitmap, selected media, job paper, every
failure before and after a document starts, and cancellation.

Windows line coverage measured 78.26 percent; the minimum rises from 74
to 77.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
PrinterWorkerHost read the console, the environment and the real
spooler directly and ended its own process when its parent was gone,
so its loop could only be run as a second process, and the lines that
process reached depended on how it was ended.

The watch on the parent is now behind IParentWatch. ParentProcessWatch
is the real one, moved unchanged, and is excluded from coverage with
its reason; it is still exercised with real processes by the existing
parent-death test. The helper loop takes the printers, the spooler's
job calls, the watch, the parent id and its two streams, and the entry
point the service calls passes the real ones. What the helper does is
the same: the same refusals, the same answers, the same exit codes,
the watch started before the request is read and stopped after the
operation. The switch over the three operations moved to a method of
its own to keep the loop short.

WindowsSpoolerJobs loses its entry point over the real spooler, which
nothing calls now that the helper passes the spooler in, and becomes
internal.

The fixture helper runs the real loop with a fake spooler that holds
one job, and can answer with text a test dictates. Tests run the loop
in process (every refusal, each operation, the error codes, redaction,
the watch) and run the service side against the fixture: what is sent,
every answer a helper can give, cancellation refusals across the
boundary, an oversized answer, and a caller that gives up.

IsolatedPrinterBackend is unchanged. Two of its lines stay uncovered:
the handler for a killed helper that has not exited after five more
seconds, which no test can produce on demand.

Windows line coverage measured 78.80 percent; by the rule the minimum
stays at 77.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The print helper read its one request line in full and only then
compared its length with a bound of 30 million characters, left over
from the time a request could carry a document. A request now holds an
operation, a job's description and the path of a staged file: a few
kilobytes. Nothing but the service starts a helper, so this was not
reachable from outside, but the helper would have read whatever it was
given into memory before refusing it.

The helper now reads the line a piece of 4,096 characters at a time,
counts as it goes and refuses with the same message as soon as more
than 1 MiB characters have arrived without a line end. That is the
bound the service already puts on the helper's answer. A request is
still the first line, ended by a line feed, a carriage return or the
end of the input; an empty first line is still not a request; input
that ends before any character is still refused as before.

Tests: a request of exactly the bound is carried out and one character
more is refused; input that never ends is refused after at most one
piece past the bound; a request that arrives a character at a time is
put together; every line ending; text after the first line is unused.
The fixture tests that start a real helper, with its input closed and
with it left open, read their requests through the same code.

The threat model's printing paragraph states the bound. The service
still applies its bound on the helper's answer after reading it whole;
that is unchanged here.

Windows line coverage measured 78.87 percent; by the rule the minimum
stays at 77.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The printer adapter refuses a staging path that is a reparse point
before it opens the file, but no test showed it: the line was reached
only through the relative-path refusal that shares it.

The test makes a directory junction inside its own temporary folder
with the system's mklink (the one kind of link any user may create),
hands its path to the adapter over a fake Windows, and asserts the
refusal and that Windows was asked nothing. The junction is removed
afterwards.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The reason on WindowsPdfRenderer said its output is pixels a test
cannot assert. That is too strong: a local run of the real renderer
against a synthetic two-page document returned the expected black and
white areas. What makes it unfit for a hermetic test is that its answer
depends on the machine: the same run returned 750 by 1125 pixels for a
request of 600 by 900, on a desktop at 125 percent. The reason now says
that.

The reason on ParentProcessWatch said all three of its outcomes are
exercised with real processes. Two are; the third (a parent that is
already gone when the helper looks for it) cannot be arranged by a
test. The reason now says which is which.

Only the two attribute texts change.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A native import declared with SetLastError replaces the thread's last error
each time it is called. The printing fakes only set it when a call failed, so a
rule that read an error one call late (after ClosePrinter, AbortPrinter or
AbortDoc) still saw the code a test had arranged, and no test could tell.

Every fake member that stands for such an import now starts by leaving a
marker, StaleLastError.Code (0x0BADC0DE), and sets the arranged error only
when it then fails. The members that stand for imports without SetLastError
(GetDeviceCaps, DeleteDC, SetStretchBltMode, DeviceCapabilities,
DocumentProperties) leave the last error alone, as the real calls do. The
fixture helper's fake spooler does the same.

One expectation changes with it. A refused SetStretchBltMode was asserted to
carry the error the test arranged; its import records none, so the exception
carries what StartPage left. That case is now its own test and expects the
marker. No product code changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The exclusion reason on ParentProcessWatch said that a parent already gone
when the helper looks for it is an outcome "which no test can arrange". It can
be arranged: a process that has exited and whose handle is still held keeps
its process id, and a fixture helper given that id exits with code 2 before it
reads a request. PrinterWorkerIsolationTests now does that, with nothing sent
to the helper and its input left open, so only the watch can have ended it.

The reason also left out a fourth outcome, which is older behaviour and is not
changed here: a lookup or wait that fails for any reason other than the parent
not running is not caught. The watch's task fails, nothing ends the helper,
the operation runs unwatched and its answer is written, and the failure comes
out only when PrinterWorkerHost.RunAsync awaits the watch in its finally
block. The reason and the member's documentation now say so, and
PrinterWorkerHostTests records what RunAsync does with a watch that failed,
using a fake.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The reason on WindowsPrinterSettings said the mode flags are decided in
WindowsPrinters, but the sizing call's mode 0 is fixed in the adapter. The
reasons on WindowsSpooler and WindowsGdiPrinter named neither the arguments
they fix nor, for the spooler, the put-back of the last error; only the member
comments did. WindowsSpoolerJobQueue had the same gap.

Each reason now lists the arguments that are fixed in the adapter (levels, null
server name, defaults, window and output file, the sizing calls' empty buffers
and mode 0, the DOCINFO and bitmap header fields) and, where a buffer is freed
after the call, that the call's error is put back as the thread's last error.
The GDI adapter says that it has nothing to put back. Text only; no code
changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A bare carriage return as a line end in the helper's request reader: the
existing "\r" cases still parsed as JSON when the carriage return was not a
line end, so two cases now put text after it that would break the request.

The stream handed to the PDF renderer: the fake renderer ignored it, so
removing the reset to position 0 passed. The fake now reads what it is handed
and a test compares it with the staged file.

Asymmetric resolution: which resolution belongs to which axis was pinned by
call order only. The resolution read from a context, the size a page is drawn
at, the media check before StartDoc and the job paper check are each run at
203 by 300 dpi.

A PDF with no pages: it is started and ended as one empty document and gets a
receipt. Recorded as it is.

A destination with no size: the fake's StretchDIBits answered the number of
rows where GDI answers 0. It now answers 0, and a page smaller than one device
pixel is shown to fail the job and abort the document. The bitmap bound test
used such a page for its lower bound and now uses one that is a full device
pixel.

No product code changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
IsolatedPrinterBackendTests.FixtureExecutable and
PrinterWorkerHost.ParentEnvironmentVariable had no documentation comment, and
the constant had one blank line after it where members are separated by
three. The three plain comments this branch added to WindowsPrinters are now
boxed comments, as elsewhere in the repository. Comments and blank lines only.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Measured 87.43% (3,833 of 4,384 lines) on this branch replayed onto
release/2.0 at 077148f, with the startup and branding seams merged. The
rule is the measured figure rounded down, less one.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes merged commit e2ca086711 into release/2.0 2026-11-11 02:08:16 +00:00
Sign in to join this conversation.