refactor(windows): put sign-in startup, the branded shortcut and the browser hand-off behind tested seams #41

2026-11-11T01:50:23Z

james.maes commented

2026-11-11 01:50:23 +00:00

Plan step 9 of the multi-server build: the sign-in startup task, the branded shortcut and the browser hand-off are split into rules (covered by tests with hand-written fakes) and thin adapters over Windows.

What moves

Sign-in startup: the task rules are separate from the Task Scheduler calls; the process starter is tested against a child fixture.
Branding: the shortcut rules are separate from the shell, the folder security and the registry; the registry record is tested against a private hive.
Browser hand-off: the trace and installation rules are separate from the user's files.
Nine member-level coverage exclusions remain, each a one-line pass-through that binds a real place, each with a written reason. No class is excluded.

Behaviour changes (install, upgrade, repair, uninstall)

A shortcut file that cannot be opened is never judged. At the first look the run stops before anything changes; an uninstall fails with the record kept, where it used to forget the shortcut.
A file that appears, or replaces ours, under the wanted name during the write is no longer overwritten.
A read-only shortcut of ours no longer fails installs, upgrades, repairs or uninstalls.
A recorded name that has become a link, or a file that is not a shortcut, is left alone, noted and forgotten; it no longer fails every run.
Leftover temporary shortcut files from an interrupted run are removed (exact name pattern only).
A registry value of the wrong kind reads as nothing recorded, with a warning.
A scheduler program that cannot be started reports "Could not start Windows Task Scheduler administration." in place of the raw Windows error.

Verification

Head 6add6ad on release/2.0 at b5dfb2c.
build/Verify.ps1 passed on that head: 773 Core, 1,305 Windows runtime and command line, 352 presentation tests, none failed; Core, presentation and command line at 100%; Windows 81.21% (3,580 of 4,408 lines).
Windows minimum in build/minimum-coverage.json raised from 73 to 80.

Review (independent reviewer, two passes)

1dedd05: seven should-fix, seven nits. All addressed in eight commits.
16f57a5: Verdict on 16f57a5: 0 blockers, 0 should-fix, 7 nits. The reviewer made 27 single changes in a scratch copy; 25 were caught by tests.
The head merged is 16f57a5 replayed onto b5dfb2c; git range-diff 6f0237d..16f57a5 b5dfb2c..6add6ad reports all 25 patches identical.

Left open, to be recorded in the audit (all nits)

The order "judge ours, then clear the read-only mark" is not pinned at the second look.
A read-only leftover temporary file fails an uninstall until the mark is cleared.
A damaged shortcut of ours under the current name refuses every install, as in 1.2.1; the header-plus-garbage case has no test.
The Windows reason a scheduler program could not start is discarded.
One timeout test depends on the child starting within six seconds.
Two changelog sentences disagree; two files keep two blank lines after their usings.

Not verified

No elevated MSI run: the real machine registry key, the custom actions, and shell and folder creation as SYSTEM are unexercised. This belongs to the upgrade qualification.
The real-shell tests ran only on this PC's Windows build.

🤖 Generated with Claude Code

Plan step 9 of the multi-server build: the sign-in startup task, the branded shortcut and the browser hand-off are split into rules (covered by tests with hand-written fakes) and thin adapters over Windows. **What moves** - Sign-in startup: the task rules are separate from the Task Scheduler calls; the process starter is tested against a child fixture. - Branding: the shortcut rules are separate from the shell, the folder security and the registry; the registry record is tested against a private hive. - Browser hand-off: the trace and installation rules are separate from the user's files. - Nine member-level coverage exclusions remain, each a one-line pass-through that binds a real place, each with a written reason. No class is excluded. **Behaviour changes (install, upgrade, repair, uninstall)** - A shortcut file that cannot be opened is never judged. At the first look the run stops before anything changes; an uninstall fails with the record kept, where it used to forget the shortcut. - A file that appears, or replaces ours, under the wanted name during the write is no longer overwritten. - A read-only shortcut of ours no longer fails installs, upgrades, repairs or uninstalls. - A recorded name that has become a link, or a file that is not a shortcut, is left alone, noted and forgotten; it no longer fails every run. - Leftover temporary shortcut files from an interrupted run are removed (exact name pattern only). - A registry value of the wrong kind reads as nothing recorded, with a warning. - A scheduler program that cannot be started reports "Could not start Windows Task Scheduler administration." in place of the raw Windows error. **Verification** - Head `6add6ad` on `release/2.0` at `b5dfb2c`. - `build/Verify.ps1` passed on that head: 773 Core, 1,305 Windows runtime and command line, 352 presentation tests, none failed; Core, presentation and command line at 100%; Windows 81.21% (3,580 of 4,408 lines). - Windows minimum in `build/minimum-coverage.json` raised from 73 to 80. **Review (independent reviewer, two passes)** - `1dedd05`: seven should-fix, seven nits. All addressed in eight commits. - `16f57a5`: `Verdict on 16f57a5: 0 blockers, 0 should-fix, 7 nits.` The reviewer made 27 single changes in a scratch copy; 25 were caught by tests. - The head merged is `16f57a5` replayed onto `b5dfb2c`; `git range-diff 6f0237d..16f57a5 b5dfb2c..6add6ad` reports all 25 patches identical. **Left open, to be recorded in the audit (all nits)** - The order "judge ours, then clear the read-only mark" is not pinned at the second look. - A read-only leftover temporary file fails an uninstall until the mark is cleared. - A damaged shortcut of ours under the current name refuses every install, as in 1.2.1; the header-plus-garbage case has no test. - The Windows reason a scheduler program could not start is discarded. - One timeout test depends on the child starting within six seconds. - Two changelog sentences disagree; two files keep two blank lines after their usings. **Not verified** - No elevated MSI run: the real machine registry key, the custom actions, and shell and folder creation as SYSTEM are unexercised. This belongs to the upgrade qualification. - The real-shell tests ran only on this PC's Windows build. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 25 commits 2026-11-11 01:50:23 +00:00

refactor(startup): separate the sign-in task rules from the Windows calls 1f1ba2fead

AgentStartup mixed the rules of the sign-in startup task with the calls
that carry them out, so 87 of its 137 measured lines could only run by
changing this computer's scheduled tasks.

The calls now sit behind ITaskScheduler: run schtasks.exe with a limit,
read who is asking, resolve an account name, read a path's attributes.
WindowsTaskScheduler is the real one and is excluded from coverage with
its reason. The rules moved unchanged to AgentStartupTasks: the task
definition, what counts as our task, who may change it, and when one is
created, replaced, removed or left alone. The folder the definition is
staged in and its two trust checks are passed in, because a temporary
file can never pass the real checks.

AgentStartup keeps its public members and passes them on. Messages,
scheduler arguments, the 15 second limit and the task XML are the same.

Tests run the rules against a scheduler that exists only in memory:
every refusal, the commands sent and their order, the staged file's
content, encoding and protection, verification of what Windows stored,
uninstall cleanup, and the scheduler failing to start or hanging.

Windows line coverage measured 72.5 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>

refactor(branding): separate the shortcut rules from the shell, folder security and registry 88c4139cb7

AgentBrandingInstaller mixed the rules of the branded all-users
shortcut with the COM shortcut object, folder security descriptors and
the registry, so 99 of its 127 measured lines could only run as an
administrator changing this computer.

Three seams now carry the calls, each with a real Windows class that
is excluded from coverage with its reason: IShortcutShell (read and
write a .lnk through WScript.Shell), IPathSecurity (who is asking, a
folder's owner and access list, creating a folder with a descriptor;
named for reuse by the updater) and IShortcutRecord (the registry value
that remembers the last shortcut). The rules moved unchanged to
AgentBrandingShortcuts: who may brand, which folders must be trusted
first, what the shortcut says, when one is written, replaced or
removed, and the access list of a new shortcut folder. The install
path, the Start menu folder, the machine state check and the branding
loader are passed in.

AgentBrandingInstaller keeps its public members and passes them on,
and keeps the naming and access-list rules that need nothing from
Windows. Shortcut properties, folder access, registry value, messages
and the order of steps are the same.

Tests run the rules in temporary folders against hand-written fakes:
refusals, untrusted folders, a foreign shortcut, rebranding, removal,
and the shell or the final move failing.

Windows line coverage measured 75.05 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>

refactor(callback): separate the hand-off trace and installation rules from the user's files ba00b1391e

The browser callback's trace and installation checks could only run
against the user's real trace file and the real Program Files folder,
so 46 of their 52 measured lines were never run by a test.

The trace file sits behind ITraceFile. DiskTraceFile is the ordinary
file and is tested in a temporary folder, so it needs no exclusion.
BrowserCallbackTrace holds the rules: one stamped line per entry, a
fresh file past 256 KiB, a write or read failure that never reaches the
hand-off, and the newest 40 entries inside a window. What the handler
records stays in QqqBrowserCallback and takes the trace and the
forwarding as arguments. The installation check takes the process path,
the Program Files folder and IPathSecurity, which gains a file reader.

The three public members keep their signatures and pass the real
places on; they are excluded from coverage with their reasons. Trace
lines, limits, messages and the checks made are the same.

A test forwards a callback carrying a canary authorization code and
state through the real client path to a real inbox and finds neither,
nor any query, in the trace; the same holds when forwarding fails.

Windows line coverage measured 76.24 percent; the minimum rises from 74
to 75.

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 76 on the current release branch 0fbd947abe

The three native-seam commits were measured on an earlier base and
raised the minimum to 75. Rebased onto the release branch after the
unreachable print paths were removed, the Windows assembly measures
77.77 percent (2,933 of 3,771 lines), so by the rule of one below the
whole percent the minimum is 76.

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

fix(branding): check the previous shortcut before writing; always record what was written bb50637a63

A branding run wrote the new shortcut, then looked at the shortcut the
last run had recorded, and failed if that file had become someone
else's or its recorded name was invalid. The new shortcut was then on
disk but not recorded, so an uninstall could not remove it, and every
later run failed the same way. The installer runs this step as a
required action, so that meant every later install and upgrade failed.

The previous shortcut is now judged before anything is written. If it
is still ours it is removed after the new one is written and recorded.
If the file under the old name is no longer ours, or the recorded name
is not a shortcut file name in the shortcut folder, it is left alone,
a note goes to the machine activity log, and the new shortcut is
written and recorded: a foreign file under the old name is no reason to
refuse this brand its shortcut. A previous shortcut that cannot be read
still stops the run, before anything has changed. The new name is
recorded before the old shortcut is deleted, so a failed delete cannot
leave the new shortcut unrecorded.

The test that pinned the old behavior is replaced by tests of the new
one, including that the record names the new shortcut and that a
second run is clean.

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

fix(callback): take the start of the recent-trace window to the millisecond 1e1751396c

Trace entries are stamped to the millisecond, but the start of the
window was compared with the clock's full precision, so an entry
written exactly one window ago fell outside its own window by the
fraction of a millisecond its stamp had dropped. The start of the
window is now truncated the same way. A test with the entry exactly at
the boundary fails on the old comparison.

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

style: end every file the branch touched with a newline 1427debe4b

Twenty files added or changed by the native-seam commits ended without
a final newline, which .editorconfig requires (insert_final_newline):
thirteen under src/Qqq.PrintAgent.Windows and seven under
tests/Qqq.PrintAgent.Cli.Tests. Only the last byte of each changes.

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

refactor(startup): pin the scheduler program in the rules; exclude only what a test cannot run df7cee276f

Four findings of the review of the sign-in startup split.

The program the rules run was chosen inside the excluded adapter, so
nothing tested that it is schtasks.exe from the Windows system folder
and never a name looked up on the search path. The rules now pass the
full path with every command, and a test pins it for the query, create,
delete and list commands.

WindowsTaskScheduler was excluded from coverage as a whole, although
two of its members only read: a path's attributes and the identifier an
account name stands for. Those two are now tested, in a temporary
folder and against accounts every Windows has. The exclusion is on the
two remaining members, each with its own reason: starting the program,
and reading the identity of the running process.

The rules called back into AgentStartup for the held definition file,
so a test of the rules ran the entry point's static initializer and
built the real adapters. That method moved into the rules unchanged.

The production staging was assembled inline in the entry point, where
nothing stood in the way of binding a check that does nothing.
ProtectedStaging.MachineSetup now names it, in covered code, and a test
pins the folder and that both checks are the machine state guard's own.

No behavior changes: the program, its arguments, the limit, the staged
file and the messages are the same.

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

refactor(branding): keep the rules out of the entry point; bind the installed places by factory d8cf297c8e

Two findings of the review of the shortcut split.

The rules called back into AgentBrandingInstaller for the file name,
the path and the Start menu access check, so a test of the rules ran
the entry point's static initializer and built the real shell, folder
security and registry adapters. Those three rules moved into
AgentBrandingShortcuts, in a second part of the class that also takes
the shortcut file and folder helpers. The entry point now holds no rule.

The production places were assembled inline in the entry point, where
nothing stood in the way of binding a check that does nothing.
BrandingPlaces.Installed now names them, in covered code, and adds the
installer mark to each note, which the excluded log writer used to add.
A test pins the paths, that the state check and the branding loader are
the real ones, and the mark.

No behavior changes: names, paths, checks, messages and log lines are
the same.

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

fix(branding): keep every owned shortcut on record; a file that is not ours no longer fails an uninstall e09d971981

Rebranding recorded the new shortcut and then deleted the previous
one without looking at it again. When that delete failed the run
failed, the old shortcut stayed on the Start menu, nothing recorded it
any more, and no later run or uninstall removed it. Uninstall failed
outright when the recorded shortcut had become someone else's file,
and the installer runs both as required steps.

The record now names every shortcut this product may own, at every
moment: the current one in LastShortcutName, as before, and any others
in a second value, PendingShortcutNames. A name is recorded before its
file is created and forgotten only after the file is gone or has been
found to be someone else's.

Branding proves the record changeable and judges every recorded
shortcut before it writes anything; one that cannot be read still
stops the run there. It then records the new name beside the ones to
remove, writes the shortcut, makes it current, and removes the earlier
ones, looking at each again immediately before the delete. One that
cannot be removed no longer fails the install: it stays recorded with a
warning in the activity log, and the next branding run or the
uninstall removes it. A failed write puts the record back as it was.

Uninstall removes every recorded shortcut that is still ours. One that
has become someone else's, or a recorded name that is not a shortcut
file name in the folder, is left alone with a warning and forgotten.
It still fails, before deleting anything, when a shortcut cannot be
read, the record cannot be changed or the folder is not trusted, and
it fails with the record kept when a shortcut of ours cannot be
deleted, so it can be run again.

That a caller who cannot change the record must fail first was an
ordering inside the excluded registry class (it opened the key
writable on read). It is now a call the rules make,
IShortcutRecord.RequireChangeable, documented on the interface and
tested. Notes are written once the run has done its work, and a note
that cannot be written does not fail the run.

Tests: RemovalRefusesAShortcutItCannotBeSureIsOurs is replaced by
tests of the new behavior and of each case that still fails; removal
tests moved to AgentBrandingRemovalTests and the shared setup to
BrandingSetup. New cases: the delete failing and the next run or the
uninstall cleaning up, a shortcut replaced between the two looks, a
record that cannot be changed or written, an existing shortcut folder
an ordinary user can write to, and a shell that fails part way and
leaves half a file. CHANGELOG and docs/BRANDING.md state the changes.

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

test(branding): write and read a real shortcut through the Windows shell; drop its exclusion b896094f46

WindowsShortcutShell was excluded from coverage with the reason that a
test cannot run it without Windows Script Host. It can: the automation
object is part of Windows. The class also held the only untested
mapping of the shortcut split, five property names written and three
read back, where one wrong name would fail every install.

A test now writes a shortcut through the real class into a temporary
folder, reads it back, and finds the working folder and the icon in the
file itself, since those two are not read back through the shell. The
exclusion is removed; the test covers every line of the class. On a
machine without the automation object the test fails and says so.

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

test(branding): run the folder security adapter in a temporary folder; exclude only the identity reads ae01d8ebd2

WindowsPathSecurity was excluded from coverage as a whole. Three of its
five members can run in a test without administrator rights: creating
a folder with a descriptor and reading a folder's and a file's
descriptor. They are now tested in a temporary folder with a
descriptor that names only the account the tests run as.

The exclusion is on the two identity reads, each with its own reason.
The reasons and the class summary now name every validator the adapter
serves, including MsiUpdateInstaller.ValidateTrustedAcl, which the old
justification left out.

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

fix(callback): read trace stamps in the clock's zone; start the trace tests' clock at a fixed instant 2bb0094660

The trace tests ran a manual clock that started at the real time in
the machine's zone, and stamps were read back as local time of the
machine. In the hour a daylight-saving change repeats, an entry reads
as an hour newer than it is, so the recent-trace tests could fail
depending on when and where they ran.

BrowserCallbackTrace now reads a stamp in the time zone of the clock
that wrote it. With the system clock that is the machine's zone, so the
installed product behaves as before. The manual clock takes an optional
start and zone; its default is unchanged for every other test, and the
trace tests start it at a fixed instant in a zone that never changes
its clocks.

The start of the recent window is now rounded down to the millisecond
after the window is taken off the present time, not before, so an
entry exactly one window old stays recent for a window with a fraction
of a millisecond too. A stamp that cannot be an instant in the clock's
zone is not recent, as before. Tests cover both.

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

style(test): end the two factory test files with a newline b8431898bb

BrandingPlacesTests.cs and ProtectedStagingTests.cs, added two commits
ago, ended without the final newline .editorconfig requires. Every
file the branch adds or changes now ends with exactly one.

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

docs(branding): say exactly when a branding run can still fail late 9ee17e4174

The summary of the shortcut rules said a run that has to fail fails
before it has changed anything. That holds for every refusal, but not
for a shortcut or a record that cannot be written, which fail after
the new name is on record. The summary now says both, and what the
record holds in each case. Comment only.

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 77 7d6127567d

The review follow-ups put the shortcut shell, three members of the
folder security adapter and two of the task scheduler adapter under
test and removed their exclusions, and added tests of the shortcut
record. The Windows assembly measured 78.23 percent (3,023 of 3,864
lines) and 78.39 percent (3,029 of 3,864) in two runs of the gate; the
difference is the timing-dependent lines. By the rule of one below the
whole percent the minimum is 77.

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

test(startup): run the process starter against a child fixture; drop the last adapter exclusions e3878f692b

Review round two, items 5 and 12.

WindowsTaskScheduler.RunAsync was excluded from coverage because it
starts schtasks.exe. Since the program became a parameter it no longer
has to: tests now give it the repository's child-process fixture, which
gains a plain stand-in mode that prints, fills its error stream, exits
with a given code or waits to be killed. Covered: a program that
cannot be started, the exit code and output of a normal run, an error
stream of a megabyte that must not hold the child up, and the kill and
what is returned at the limit.

One behavior changes with it. The interface says a program that could
not be started is reported as null, but Process.Start throws instead
of returning null, so a missing schtasks.exe surfaced as a raw Windows
error. It is now reported as not started, and the rules say "Could not
start Windows Task Scheduler administration." as they always meant to.

CurrentCaller, IsAdministrator and CurrentUserSid were excluded as
reads a test could only repeat. A test can call them, which is the
standard's criterion, so each has a test of what holds on any machine:
a well-formed identifier that is the process token's, and an
administrator flag that agrees with the token's enabled groups. The
three exclusions are removed; neither adapter has one left.

The held definition file asked the real machine state guard for its
folder directly. It now asks the staging it was given, as the rest of
the rules do; the installed product's staging is that same guard.

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

test(branding): run the registry record against a private hive; drop its exclusion c7544a5636

Review round two, item 4.

RegistryShortcutRecord was excluded from coverage and had never run:
only an elevated install could reach it, and a same-brand upgrade
would never write or read the new multi-string value. It now takes
its base key and key path. The installed product takes
RegistryShortcutRecord.Machine, which binds this product's key in the
machine hive in the 64-bit view, and a test pins that. The tests give
the real class a hive file of their own in a temporary folder, loaded
with RegLoadAppKey, so no hive of this computer or of a user is read
or written. The native call is in the test project.

Covered: a missing key and missing values, the two value names and
kinds, the string a 1.2.x install wrote read back and left as it is
beside the installer's label values, the proof of changeability
creating the key and nothing else, forgetting, and the two cases the
old class left to chance.

A value of a kind the record does not write was read as nothing
recorded without a word. It still reads as nothing recorded, and now
gives a warning that names the value, on every such read, until the
record is written again. An expandable string under the current name
is read as it is stored: no environment name in it is expanded.

The warning goes where the notes go. Writing a note without failing
the caller moved from the rules to BrandingPlaces.TryNote so that the
record and the rules share it. The class is no longer excluded.

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

fix(branding): never judge a shortcut that cannot be opened; a file that is no shortcut is not ours 40368e1de0

Review round two, items 1, 9 and 10.

The Windows shortcut object does not fail on a file it cannot open. A
real shortcut another program holds without sharing, an empty file and
a file of other content all read back with empty values. The rules
took empty values for someone else's shortcut, so a shortcut of ours
that was merely held was left on the Start menu and forgotten for
good: an uninstall in that moment succeeded, logged "no longer belongs
to this application" and cleared the record. The shell fake hid it by
throwing where the real shell answers. A same-brand upgrade with the
current shortcut held said a different shortcut used the name.

WindowsShortcutShell.Read now opens the file itself first and keeps it
open, shared for reading, while the shortcut is read. A file that
cannot be opened fails as the file system fails. A file that opens
and does not begin as a shortcut file does is reported as no shortcut
(null), apart from a shortcut that says something else. A call the
automation object refuses is one kind of failure that names the call.

What the rules make of each, at each look:
- A file that cannot be opened is never judged. At the first look the
  run stops before anything is changed, saying which shortcut could
  not be read. At the look before a delete the file is left and its
  name stays on record with a warning, like a delete that failed; an
  uninstall fails with the record kept and can be run again.
- A file that opens and is no shortcut at all is not ours: left
  alone, noted, forgotten; under the wanted name it is not replaced.
- A recorded name that has become a link is not ours either, since
  the product never writes one, and is not opened. It used to fail
  every install and uninstall. IPathSecurity.IsLink reports the fact.

The second look no longer catches every exception: it catches what the
file system and the shell throw and puts the reason in the warning.

The shell fake now opens the file the way the real shell does and
reads anything but its own text as no shortcut. Real-shell tests in a
temporary folder cover a held file, an empty file, a file of other
content, a shortcut cut off in its header and a save Windows refuses;
AgentBrandingRealShellTests runs the rules with the real shell through
the held-shortcut cases the review reproduced.

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

fix(branding): look at the destination again before the move; clear a read-only mark only on our own shortcut de3d9ae7b0

Review round two, items 2 and 3. Both are in 1.2.1 as well.

The new shortcut was moved over the destination with overwrite, on
the strength of one look taken before the shortcut was written. A
shortcut someone else put under the wanted name in between was
overwritten, by code that runs as SYSTEM, both when a shortcut of ours
had been there and when nothing had. The destination is now looked at
again immediately before the move. When nothing was there at the
first look the move never overwrites, and a file that has appeared is
refused. When a shortcut of ours was there it is replaced only if the
second look still finds a shortcut of ours; one that cannot be opened
then is not replaced and the run says it could not be read.

A read-only mark on a shortcut of ours failed the move of a
same-brand install, upgrade or repair, and the delete of a rename or
an uninstall, every time, until someone cleared it by hand. The mark
is now cleared once the file has been found to be ours, immediately
before it is replaced or deleted. A file that is not ours keeps its
mark and is never changed.

When a write fails, the record is put back as found unless a shortcut
of ours was under the new name at the first look; that replaces the
check for whether a file exists there now, which a file of someone
else's would have satisfied.

Tests: the two overwrite cases and a held destination with the hooked
shell fake; read-only shortcuts of ours and of someone else's on real
files, with the fake and with the real shell.

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

fix(branding): remove temporary shortcut files an earlier run left behind e8ea7d1532

Review round two, item 8.

The shortcut is written to .qqq-<identifier>.lnk and moved into
place. A run killed between the two, or one that could not delete the
file afterwards, left it visible on the Start menu under a name
nothing records, and nothing ever removed it.

A branding run now removes such files once it is clear to go, before
it writes anything, and an uninstall removes them before it deletes
the recorded shortcuts. Only a file whose name is exactly the
temporary pattern is touched; the name is never recorded. A branding
run that cannot remove one notes it with the reason and goes on; an
uninstall that cannot fails with the record kept, like a shortcut of
ours it cannot delete, so it can be run again.

An uninstall with nothing on record touches nothing, so the record
must not be empty while a temporary file exists. One path could make
it so: a first install whose write failed and whose temporary file
could not be deleted put the record back to empty. A failed write now
leaves the new name on record while a temporary file is in the folder.

Tests: a leftover among six files with lookalike names, for a run and
for an uninstall; a refused run leaving it alone; a held leftover
noted and then removed; an uninstall failing over one and succeeding
when run again; and the record not emptied while one exists.

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

test(branding): follow every interrupted state of the record with an install and an uninstall 98accec1be

Review round two, item 6.

No test covered an uninstall when only other names are on record,
which is what a first install leaves when it is killed after the
shortcut is moved into place and before its name is made current.
Changing the uninstall to do nothing without a current name passed
every branding test; it now fails three.

AgentBrandingRecordStateTests arranges each state a killed or failed
run can leave, and the one a 1.2.x install leaves, and follows it once
with an install of the wanted brand and once with an uninstall: both
names on record with only the earlier shortcut on disk, with both on
disk, with the new one already current, only an other name on record
with and without its shortcut, and one current name alone. Each must
end clean. The state with an earlier shortcut that still cannot be
deleted is followed through both ways until it can.

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

docs(branding): state what the shortcut step does after the review; document and space the fields ec94f6db36

Review round two, items 7 and 11.

The changelog entry claimed more than the code did: that every
shortcut is recorded until it is gone (not the temporary file), that a
recorded shortcut that cannot be read still fails a run (a held or
damaged file did not), that a run always fails before it has changed
anything when the record cannot be written (only the proof made up
front does), and that a failed write leaves the record as it was (not
when a shortcut of ours already has the name). The entry is rewritten
to what holds now: what counts as ours, what a file that cannot be
opened means at each look, the second look at the name before the
shortcut is put in place, the read-only mark, the temporary files, the
registry value kinds, and each way a run can still fail and what the
record holds then. It also declares the sign-in startup message for a
scheduler program that cannot be started. docs/BRANDING.md says the
same, and the summary of the rules class states the ownership and
record rules in one place.

Layout: private constants and fields in the files this branch touches
have a documentation comment, and documented fields are separated like
other members. Comments and blank lines only.

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

docs(branding): name everything the entry point binds in its exclusion reasons d966ccb522

The two entry-point members that run the branding rules said their
bindings are tested in BrandingPlaces.Installed. Since this round they
also bind the registry record through RegistryShortcutRecord.Machine,
and the shell and folder security adapters are tested against Windows.
The reasons now say so. Attribute text only.

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 80 on the current release branch 6add6adbeb

Measured 81.19% (3,579 of 4,408 lines) on this branch replayed onto
release/2.0 at 6f0237d. 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 077148ff41 into release/2.0

2026-11-11 01:50:44 +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#41