refactor(windows): put the in-app update, the service controls and the pipe's identity check behind tested seams #52

2026-11-11T06:51:21Z

james.maes commented

2026-11-11 06:51:21 +00:00

Plan step 8 of the multi-server build: the in-app update, the service controls and the pipe's identity check are split into rules (covered by tests with hand-written fakes) and thin adapters over Windows. A restructuring: the installed product behaves as before.

What moves

Update installer (MsiUpdateInstaller*, MsiUpdateSteps*, MsiUpdatePlaces): the staging checks, the hand-off to the helper, running Windows Installer on a verified MSI, the exit-code and three-program version rules, the wait for the companion by process identity.
Update service (MsiUpdateService): download, pause, hand-off, and what is undone on a failure or a cancellation.
Service controls (AgentServiceControl, AgentServiceActions): the status mapping, start and restart with their waits.
Pipe identity check (AgentPipePeer, IPipePeerFacts): whether the process serving the management pipe is the installed service running as SYSTEM. This is what the companion and the command line rely on before they send anything.
Six one-line members remain excluded from coverage, each binding a real place (the real installer, the real service, the Services console), each with a written reason.

What does not change

The Windows Installer command line; the files, owners and access lists checked and the order they are checked in, all before the installer starts; the hold on the MSI while it runs; what an exit code means; the calls to the service control manager; the conditions of the identity check, their order and its two refusals. The reviewer derived this from the old and new code independently and found no difference. Four members of MsiUpdateInstaller are no longer public; nothing outside the Windows library called them.

Coverage

Windows line coverage measures 96.06% (4,539 of 4,725 lines) on this head. The minimum in build/minimum-coverage.json rises from 87 to 95. The assembly is now above its release bar of 90.

Verification

Head c10b0ec on release/2.0 at 4e01519.
build/Verify.ps1 passed twice on that head: 1,174 Core, 2,321 Windows runtime and command line, 360 presentation tests, none failed; Core, presentation and command line at 100%.

Review (independent reviewer, three passes)

ef98868: 0 blockers, 7 should-fix, 6 nits. No behaviour change found; the should-fix items were missing tests (for example nothing failed when production was bound to the wrong folder, or when the peer's process handle was not held).
940d174: 0 blockers, 0 should-fix, 3 nits. Fourteen of the reviewer's seventeen surviving changes are now caught by tests; three need an elevated run or a file symbolic link and are recorded.
c10b0ec, the head merged: Verdict on c10b0ec: 0 blockers, 0 should-fix, 2 nits. The reviewer found the minimum of 95 safe: the lowest of six runs is 50 lines above the gate, against a run-to-run spread of 4 lines. The two nits are wording in the audit (a row to add for the one binding that can only be checked by reading; one header sentence) and go into the next documentation change.
940d174 was moved onto 4e01519 as 35ad5cc: every code, test and build file byte-identical; its five audit rows renumbered W77 to W81.

Recorded for the upgrade (ADR-0004, docs/TODO.md)

The upgrade from 1.2.x is driven by 1.2.1's own updater. After its installer exits it reads the three programs' versions, writes the outcome, sends one version 1 request (resume-after-update) after its own identity check, and starts the companion. 2.0 must keep the three program paths, the service name and account, and keep answering that request; an unanswered request costs up to 35 seconds before the companion reopens. The step that retires version 1 of the pipe must keep it.

Not proven here

Nothing ran elevated. Only an elevated run on an installed product can show: a real authentication through the new adapters; a real msiexec run; the real Services console; the service controls on the real service. These are in the upgrade qualification entry.

Left open, in the audit

W77 to W81: whose account is read cannot be told without elevation; a status refresh that does nothing would pass the tests; two service handles per authentication are left to the finalizer; an empty session folder is left when the second folder check fails; the link check of last-result.json needs a file symbolic link to test.

🤖 Generated with Claude Code

Plan step 8 of the multi-server build: the in-app update, the service controls and the pipe's identity check are split into rules (covered by tests with hand-written fakes) and thin adapters over Windows. A restructuring: the installed product behaves as before. **What moves** - **Update installer** (`MsiUpdateInstaller*`, `MsiUpdateSteps*`, `MsiUpdatePlaces`): the staging checks, the hand-off to the helper, running Windows Installer on a verified MSI, the exit-code and three-program version rules, the wait for the companion by process identity. - **Update service** (`MsiUpdateService`): download, pause, hand-off, and what is undone on a failure or a cancellation. - **Service controls** (`AgentServiceControl`, `AgentServiceActions`): the status mapping, start and restart with their waits. - **Pipe identity check** (`AgentPipePeer`, `IPipePeerFacts`): whether the process serving the management pipe is the installed service running as SYSTEM. This is what the companion and the command line rely on before they send anything. - Six one-line members remain excluded from coverage, each binding a real place (the real installer, the real service, the Services console), each with a written reason. **What does not change** The Windows Installer command line; the files, owners and access lists checked and the order they are checked in, all before the installer starts; the hold on the MSI while it runs; what an exit code means; the calls to the service control manager; the conditions of the identity check, their order and its two refusals. The reviewer derived this from the old and new code independently and found no difference. Four members of `MsiUpdateInstaller` are no longer public; nothing outside the Windows library called them. **Coverage** - Windows line coverage measures 96.06% (4,539 of 4,725 lines) on this head. The minimum in `build/minimum-coverage.json` rises from 87 to 95. The assembly is now above its release bar of 90. **Verification** - Head `c10b0ec` on `release/2.0` at `4e01519`. - `build/Verify.ps1` passed twice on that head: 1,174 Core, 2,321 Windows runtime and command line, 360 presentation tests, none failed; Core, presentation and command line at 100%. **Review (independent reviewer, three passes)** - `ef98868`: `0 blockers, 7 should-fix, 6 nits`. No behaviour change found; the should-fix items were missing tests (for example nothing failed when production was bound to the wrong folder, or when the peer's process handle was not held). - `940d174`: `0 blockers, 0 should-fix, 3 nits`. Fourteen of the reviewer's seventeen surviving changes are now caught by tests; three need an elevated run or a file symbolic link and are recorded. - `c10b0ec`, the head merged: `Verdict on c10b0ec: 0 blockers, 0 should-fix, 2 nits.` The reviewer found the minimum of 95 safe: the lowest of six runs is 50 lines above the gate, against a run-to-run spread of 4 lines. The two nits are wording in the audit (a row to add for the one binding that can only be checked by reading; one header sentence) and go into the next documentation change. - `940d174` was moved onto `4e01519` as `35ad5cc`: every code, test and build file byte-identical; its five audit rows renumbered W77 to W81. **Recorded for the upgrade (ADR-0004, `docs/TODO.md`)** - The upgrade from 1.2.x is driven by 1.2.1's own updater. After its installer exits it reads the three programs' versions, writes the outcome, sends one version 1 request (`resume-after-update`) after its own identity check, and starts the companion. 2.0 must keep the three program paths, the service name and account, and keep answering that request; an unanswered request costs up to 35 seconds before the companion reopens. The step that retires version 1 of the pipe must keep it. **Not proven here** Nothing ran elevated. Only an elevated run on an installed product can show: a real authentication through the new adapters; a real `msiexec` run; the real Services console; the service controls on the real service. These are in the upgrade qualification entry. **Left open, in the audit** - W77 to W81: whose account is read cannot be told without elevation; a status refresh that does nothing would pass the tests; two service handles per authentication are left to the finalizer; an empty session folder is left when the second folder check fails; the link check of `last-result.json` needs a file symbolic link to test. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 17 commits 2026-11-11 06:51:21 +00:00

refactor(windows): put the update installer's steps behind process and folder-security seams 7b7bdac070

The steps of a machine update (the staging checks, preparing the helper,
the hand-off, applying the MSI and recording the outcome) move from the
static MsiUpdateInstaller into MsiUpdateSteps. The steps reach Windows
only through IPathSecurity, which sign-in startup and branding already
use, and the new IProcessHost: the current process, a process by its id,
a program started and a file's product version. The folders, the machine
state check, the log, the pipe and the clock are given in MsiUpdatePlaces.

No rule changes. The Windows Installer command line, the files, owners
and access lists checked and their order, the companion identity check
and its 30 second wait, the exit-code handling and the three closing
steps are the code that was there, with the direct Windows calls replaced
by the seam. MsiUpdateInstaller keeps ApplyAsync and
ReadLastOutcomeMessage as its entry points and the rules that need
nothing from Windows; PrepareHelper, StartHelper, ValidateStagingDirectory
and UpdatesDirectory are no longer public, since only MsiUpdateService
called them.

Tests run every step in temporary folders against FakePathSecurity and a
new FakeProcessHost, with one test per refusal and the installer as a
process of the fake; the real process adapter runs against the worker
fixture's stand-in. The two entry points that stay excluded from coverage
only pass a call on to steps bound to this computer.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(windows): give the update service its release source, pipe and update steps 8aec71ed9c

MsiUpdateService built its release client, called the service's pipe and
the installer through statics, and read the process path and id itself,
so none of its flow could run in a test. It now takes an
IMsiReleaseSource (which GitHubMsiReleaseClient implements), the pipe as
a function and the update steps; the public constructor the companion
uses binds the GitHub feed, the installed service's pipe and the
installed product's steps, as before.

The flow is unchanged: check, download into a session folder that is
validated before and after it is created, prepare, ask the service to
pause, start the helper, close the companion; lift the pause only when
it was asked for and remove what was staged unless the helper already
runs from it.

Tests run the flow over the real update steps in temporary folders with
a fake release source and pipe: the order of what reaches the outside,
each failure and cancellation point and what is undone after it, and the
session that is kept when it holds a junction or can no longer be
trusted.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(windows): put the service controls behind a service-manager seam 8c5dea8611

AgentServiceControl read, started and stopped the installed service
through ServiceController directly and waited on the stopwatch, so its
status mapping and its 30 second wait had no tests. The actions move to
AgentServiceActions, which holds the service through IServiceManager,
starts the Services console through IProcessHost and measures the wait
on a TimeProvider. AgentServiceControl binds the real ones and the
installed service's name.

The calls made to the service control manager and their order, the
250 ms poll, the 30 second limit and every message are unchanged.

Tests run the actions against a service that exists only in memory and a
manual clock. The real adapter is read against the built-in remote
procedure call service and its controls are sent only to a name no
service has.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(windows): decide pipe-peer trust from facts behind a seam 33596ed76a

AgentPipePeer asked the kernel, the service control manager and the
process for its facts inline, so only the wording of a refusal could be
tested. The facts now come through IPipePeerFacts (which process serves
the pipe, a service's state and process, a process's account and
program) and the decision stays in AgentPipePeer, in the same order and
with the same two refusals. The service and the process are held until
the connection ends, as before.

Tests go through each way the other end is not the installed service,
each fact that cannot be read, and each way the service can go away
during the check, and show that everything acquired is released. The
real facts are read from a pipe the test serves itself, the test
process, a harmless child program and a read-only status query of a
built-in service.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

build(coverage): raise the Windows minimum to 88 9ece27b64c

The update installer, the update service, the service controls and the
pipe-peer check are now run by tests. Merged line coverage of
Qqq.PrintAgent.Windows measured 89.11% (3,969 of 4,454 lines) on the code
of this branch; by the project rule
the minimum is the measured value rounded down, less one point.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs: record the update, service-control and pipe-peer seams 142244cf02

The changelog says what moved and that the installed product behaves as
before. The development guide gets a section on testing code that
reaches Windows: the three parts, the seams and fakes of this step, and
the rules for fakes, real adapters and coverage exclusions. The threat
model records what the update and pipe-peer tests now show and what
still needs an installed, elevated qualification.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

build(coverage): raise the Windows minimum to 94 on the current release branch 21cb4add62

Measured 95.37% (4,230 of 4,435 lines) on this branch replayed onto
release/2.0 at 65a99d3, with the printing seams merged underneath. The
rule is the measured figure rounded down, less one. The release bar for
this assembly is 90.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

refactor(windows): bind the installed product through values the tests compare dfb63f8cea

The review of this step found that the four lines that bind production to
the real places had no test: bound without the machine state guard, to a
pipe that always answers "Ready for update.", to update steps over another
Program Files folder, or to the caller's own program as the trusted
service, the whole suite still passed.

Each binding is now a value a test reads, and the entry point passes
nothing of its own, so a wrong binding cannot sit in a line that is
excluded from coverage:

- MsiUpdateSteps.Places is compared with MsiUpdatePlaces.Installed().
- MsiUpdateService.Send and .Steps are compared with the installed
  service's pipe and with MsiUpdateInstaller.Installed.
- AgentPipePeer.Installed is a check bound to the Windows facts and the
  service program under Program Files; Authenticate(pipe) and the
  two-argument Refusal go through it. A bound check is also run over a
  fake, to show that it asks its own facts and trusts its own program.

The two exclusion reasons on MsiUpdateInstaller now say where the binding
is asserted. The product does what it did: the same facts, the same
program path and the same order of checks.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(windows): show on the real adapters that the peer's process is held and whose pipe it is f7f5a772ec

Two changes to the real adapters passed every test.

Reading the peer's account through a second process object that is let go
at once removes what keeps Windows from reusing the process id during a
check. The test kept the child alive through its own handle and could not
tell. It now lets go of the handle the start gave it, ends the child by
its id and start time, and asks Windows directly: while the adapter holds
the process the id still names the ended child, and once the adapter has
let go it does not.

Asking for the pipe's client in place of its server also passed, because
both ends were the test process. The worker fixture's stand-in gets a
third behavior, "connect NAME", which connects to a pipe the test serves
under a name of its own and waits; the test asks about that connection
and expects its own id, not the child's.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(windows): keep two fakes off the machine's disk and end a stand-in when a test fails early 852062de33

- FakeProcessHost asked the real disk whether msiexec.exe and mmc.exe
  exist, so the apply and console tests needed a Windows that has both.
  The two system programs the rules start are now taken to exist; the
  disk is asked only about files a test made in its temporary folders.
- FakePathSecurity returned a trusted descriptor for a folder that does
  not exist, where Windows fails. It now fails the same way, as it
  already did for a file.
- In WindowsProcessHostTests a stand-in that never announced its id was
  left waiting forever. The start is now inside try/finally, and the
  child is ended through the start time the adapter's handle reports.

Tests only.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(windows): add the refusing tests three rules lacked and pin three orders faae0c93be

Rules that no test refused:

- The MSI must sit directly in the request's folder: a row with the MSI
  in a folder below it.
- The request file is created new: a request left in the session fails
  the preparation and stays as it was.
- A look-up of the pipe's server process that fails as an invalid
  operation is neither let through as it is nor taken for a process that
  has ended: it is the "not the installed service" refusal with the cause
  inside.

Orders and boundaries the product has and no test held:

- The MSI is checked and held once the companion has closed, not while
  it is waited for.
- A service found running at the look on the 30th second is in time: the
  state is looked at before the limit.
- The second look at the service compares only the process that hosts it.

Tests only. Each was shown to fail with the one change it guards against.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs: say which case each ended-process failure stands for, and correct two statements cfe3d9c313

The process fake, a test and the development notes said that .NET 10
reports the main module of an ended process as Windows error 299 and not
as an invalid operation. That is true only while another handle keeps the
ended process. With no other handle, which is the update helper's own
situation, the read fails as an invalid operation ("has exited"), and the
helper then takes the companion for closed. Both cases already had a
test; the fake, the two tests and docs/DEVELOPMENT.md now say which case
each stands for.

docs/security/threat-model.md said that exit code 0 is accepted when the
installed companion's version matches. The check is, and was, on the
companion, the service and the command line tool, with identical build
metadata. The paragraph on the pipe-peer check now also says what the
real readers are shown to do and the one thing only an elevated run can
show.

docs/DEVELOPMENT.md said service reads go to a built-in service only. An
older suite, AgentPipeSecurityTests, runs the real peer check on a pipe
the test serves, which asks Windows for the print service by name; the
notes now say what that reads on a computer with the product installed.
They also name the bindings a test compares.

No code changes: comments and documents.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs: record what the review of the seams left open and what 1.2.1's updater needs from 2.0 6434a6e186

docs/AUDIT-2026-11-10.md gets five open rows, W77 to W81: two changes to
the real adapters that only an elevated run can catch (the peer's account
read from the calling process; a service refresh that does nothing), the
two service handles each authentication leaves to the finalizer, the
empty session folder left when its second check fails, and the link check
of the recorded outcome, which needs a file symbolic link to show. Row R1
states the Windows minimum as it now is.

docs/adr/0004-multi-server-runtime.md records a constraint on the step
that retires version 1 of the pipe. An upgrade from 1.2.x is finished by
the updater of 1.2.x, so 2.0 has to keep what that helper needs after the
installer exits: three program paths with coherent versions, the service
name and account, the form of the recorded outcome, and an answer to the
one version 1 request it sends, resume-after-update. The section lists
what the helper does, read at v1.2.1, and what an unanswered request
costs as the code stands: the pause is not stored, so a service the
installer replaced starts without it.

docs/TODO.md adds both to the upgrade qualification, with the review's
list of what only an elevated run on the installed product can show.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(coverage): state the Windows coverage as measured, above its release bar 35ad5ccfae

CHANGELOG.md said the Windows minimum rises from 80 to 88. On the release
branch it rises from 87 to 94. With this change the library measured
between 95.69% and 95.82% of its lines over three runs of
build/Verify.ps1 on the same code (4,358 to 4,364 of 4,554; a few lines
in timing-dependent waits differ between runs), which is above its
release bar of 90 for the first time. The rule gives 94 for each of the
three (the measured figure rounded down, less one), so
build/minimum-coverage.json stays as it is.

docs/adr/0003-version-2.md, CONTRIBUTING.md, docs/SESSION-STATE.md and
row R1 of the audit described the Windows layer as below its bar with a
minimum of 86; each now says what is measured. ADR-0003's coverage rule
gains one sentence for an assembly above a bar that is below 100: the
minimum keeps following the rule and never drops below the bar once it
has been reached. That sentence was taken on 2026-11-10 under the owner's
standing instruction for this work, and is marked so.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(windows): let the pipe client call the installed peer check itself 936529b433

AgentPipePeer.Authenticate(pipe) passed the call on to the bound check,
AgentPipePeer.Installed. The review of this step showed that such a line
can pass a program of its own to the three-argument overload (the
caller's own program, in the reviewer's change B04b) and that every test
still passes, because no test may reach the installed service.

The member is removed. AgentPipeClient now calls
AgentPipePeer.Installed.Authenticate, so the Windows facts and the
service program are named in one place, the value that
InstalledProductsCheckIsBoundToWindowsAndTheServiceProgramUnderProgramFiles
reads. The decision, its conditions, their order and the two refusals are
not touched; the one test that used the removed member calls the bound
check.

docs/DEVELOPMENT.md said a wrong binding cannot hide in an entry point's
line. It now says what holds: the peer check has no pass-through, the
other entry points are one line each that names no place, and whether
such a line or the client's call uses the bound value is checked by
reading, since no test can tell.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(windows): keep the stand-in to its own checkout's child and its own tests' pipes a1c0733815

Two nits from the review of this step, both in test code.

EndStandIn recognized the child to end by the stand-in's process name
and its start time. It now also compares the program path with the
stand-in built beside these tests, so a stand-in that another checkout's
tests run at the same moment is left alone.

The stand-in's connect mode took any pipe name. It now refuses a name
that is not qqq-peer-facts- followed by letters and digits only, before
it connects to anything, with exit code 64. The rest of the name is
restricted too because .NET makes a pipe name into a path: a name that
begins as the tests' do and goes on with a separator and two dots names
another pipe. StandInRefusesToConnectToAPipeThatIsNotOneOfTheTests shows
the refusal for a name with another beginning and for two such names;
none of them is served by anything, and none is the product's.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

build(coverage): raise the Windows minimum to 95 and state the coverage as measured c10b0ec753

This branch now sits on release/2.0 at 4e01519, which has pull requests
50 and 51, and carries the three nits from its review. Four runs of
build/Verify.ps1 on that code measured the Windows library between
96.06% and 96.14% of its lines (4,539, 4,540, 4,540 and 4,543 of 4,725;
the gate cuts the figure off after two decimals), with 1,174 Core, 2,321
Windows and command-line and 360 presentation tests and Core at 100% of
2,311 lines.

The rule (the measured figure rounded down, less one) gives 95 for each
of the four runs, so build/minimum-coverage.json goes from 94 to 95. On
the release branch the minimum is 87. The library is above its release
bar of 90.

CHANGELOG.md, docs/adr/0003-version-2.md, CONTRIBUTING.md,
docs/SESSION-STATE.md and row R1 of the audit gave the minimum as 94 and
the figures of the branch's earlier base (95.69% to 95.82%, 4,358 to
4,364 of 4,554); each now gives these. Nothing else in them is changed.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

james.maes merged commit 2e08344d5f into release/2.0

2026-11-11 06:51:40 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qqq-print-agent#52