Skip to content

feat: add the windows-full manifest variant - #49

Draft
timo-tigges wants to merge 5 commits into
plannotator:mainfrom
timo-tigges:feat/windows-full
Draft

timo-tigges wants to merge 5 commits into
plannotator:mainfrom
timo-tigges:feat/windows-full

Conversation

@timo-tigges

Copy link
Copy Markdown

Draft, against docs/windows-full-acceptance.md. Covers §1 and the staging half of §2; §3–§6 are not attempted yet and nothing here claims them. Opening early so the Windows CI runs and so the two questions at the bottom can be settled before I write the rest.

Follows up #43. Measurements behind the design choices are in this comment; the short version is below.

What this adds

  • windows-full/herdr-plugin.tomlplatforms = ["windows"], min_herdr_version = "0.9.0", root's plugin version, full action parity with the root manifest.
  • windows-full/scripts/fetch-plannotator-tui.ps1 — stages the TUI to windows-full/bin/.
  • scripts/fetch-plannotator-tui.ps1 — optional -DestinationDirectory. Omitted, behaviour is unchanged.
  • check_windows_full in scripts/test-windows-full-manifest.py. Existing root, Lite and development assertions untouched.
  • windows-full/** in the CI path filters; windows-full/bin/ in .gitignore, which the root bin/* pattern does not cover because it is anchored.

The root and lite/ manifests are unchanged, so Windows on 0.8.x keeps installing Lite and macOS/Linux keeps the Unix wrapper and its gates.

Why 0.9.0, and why direct argv

Measured on Herdr 0.9.0, Windows 11 22621, x86_64. A pane program written ./bin/<name>.exe resolves against the plugin root while the review folder is passed separately as the pane cwd — Herdr's refusal for a missing program names the path it handed CreateProcessW:

CreateProcessW `"\\?\<plugin root>\bin\nope.exe editor"`
  in cwd `Some("<review folder>")` failed: os error 2

The same message shows the resolved program arriving extended-length. A native binary runs from a \\?\ path unchanged; a powershell.exe image started that way exits 0xFFFF0000 immediately. Isolated on one binary, only the prefix differing:

Program as written Exit
C:\…\bin\probe.exe 0
\\?\C:\…\bin\probe.exe 4294901760
./bin/probe.exe (and .\bin\, bin/, bin\) 4294901760

So "no launcher between Herdr and the TUI" is not a style rule on Windows — a wrapped pane cannot stay up. plannotator-tui being native is what makes the direct-argv doc pane work.

Layout

The native runtime is shared rather than duplicated, reached at ../bin/herdr-annotate.exe the way lite/ reaches it, so annotations and archives keep one location across variants. Only plannotator-tui is staged inside the variant, since the root must keep staging its own copy for Unix. Confirmed by running both fetchers from windows-full/: herdr-annotate.exe lands in root bin/, plannotator-tui.exe in windows-full/bin/.

Verified locally (Windows 11, x86_64, Windows PowerShell)

  • Variant fetch stages a checksum-verified 0.8.0 TUI; --version reports plannotator-tui 0.8.0; a second run is an idempotent no-download success.
  • Shared fetcher with no argument stages to root bin/ as before; test-fetch-plannotator-tui.ps1 -Case LocalOverride and -Case Download both exit 0.
  • check_windows_full rejects all six mutations tried: a platform gate on an action, a wrapper in the doc pane, min_herdr_version 0.8.0, a renamed action title, an unshared native runtime, and a build gated to macOS/Linux.

Not covered

§2's checksum, rollback and architecture matrix routed through the wrapper; §3's runner proof beyond the manifest step; §4; §5's README wording; §6. Native ARM64 in §4 is not closeable on my hardware, which is x86_64 — flagging that early rather than at review.

Two questions

  1. Onboarding sections 7/8/9 are referenced by §1, §3 and §6 but I cannot find that document in the repo. Is it published somewhere, or could it be added? Several boxes cite it directly.
  2. §2's staging route. I went with an optional -DestinationDirectory on the shared fetcher plus a thin wrapper that derives both the fetcher and the destination from its own location, so the build depends on neither the caller's cwd nor HERDR_PLUGIN_ROOT, and other callers keep the old default. If you would rather the shared fetcher stayed untouched, or want the destination derived differently, say so — it changes which regression tests I add next, so I would rather not write them twice.

Adds windows-full/herdr-plugin.toml against docs/windows-full-acceptance.md,
covering §1 and the staging half of §2. The root and lite/ manifests are
untouched, so Windows on Herdr 0.8.x keeps installing Lite and macOS/Linux keeps
the Unix wrapper and its gates.

The variant starts plannotator-tui as the pane process itself, which is what
requires Herdr 0.9.0. Measured on 0.9.0 (Windows 11, x86_64): a pane whose
program is written "./bin/<name>.exe" is resolved against the plugin root while
the review folder is passed separately as the pane cwd. Herdr's own refusal for
a missing program is the evidence, naming the path it handed CreateProcessW:

  CreateProcessW `"\\?\<plugin root>\bin\nope.exe editor"`
    in cwd `Some("<review folder>")` failed: os error 2

That also settles why no launcher may sit in between. The resolved program
reaches CreateProcessW in extended-length form, and while a native binary runs
from a \\?\ path unchanged, a powershell.exe image started that way exits
0xFFFF0000 at once. Isolated on one binary with only the prefix differing: the
plain absolute spelling exits 0, the \\?\-prefixed one 0xFFFF0000, and all four
relative spellings reproduce the prefixed result. So the direct-argv rule is not
a style preference on Windows; a wrapped pane cannot stay up.

The native annotation runtime is shared rather than duplicated, reached at
../bin/herdr-annotate.exe the way lite/ reaches it, so annotations and archives
keep one location across variants. Only plannotator-tui is staged inside the
variant, because the root must keep staging its own copy for Unix.

For that staging, scripts/fetch-plannotator-tui.ps1 gains an optional
-DestinationDirectory. Omitted, the destination is unchanged, so existing
callers behave exactly as before and both Windows fetcher cases still pass. A
thin windows-full/scripts/fetch-plannotator-tui.ps1 supplies it, deriving the
shared fetcher and the destination from its own location so the build depends on
neither the caller's cwd nor HERDR_PLUGIN_ROOT, and propagating the warn-and-
exit-zero contract so Lite stays available when a fetch fails. The release pin
stays single and at the repository root.

test-windows-full-manifest.py gains check_windows_full, which compares action
ids, titles, descriptions and contexts against the root manifest as sets rather
than restating them, and asserts the three panes and their shapes, the
direct-argv doc pane, the shared-runtime spellings, the absence of any inherited
platform gate, and link-handler parity. The existing root, Lite and development
assertions are unchanged. Verified against five mutations -- a platform gate on
an action, a wrapper in the doc pane, min_herdr_version 0.8.0, a renamed action
title, and an unshared native runtime -- each of which it rejects.

windows-full/** joins the CI path filters, and .gitignore learns
windows-full/bin/, which the root pattern did not cover because it is anchored.

Not yet covered, and not claimed: §3's runner proof beyond the manifest step,
§4's human qualification, and §5's README wording. Native ARM64 in §4 cannot be
closed on the hardware this was measured on, which is x86_64.
A build gated to macOS/Linux would stage nothing for this variant, leaving both
manifest programs pointing at binaries that were never fetched -- the failure
the variant exists to prevent, and one the command-equality check alone does not
see. Verified against a macos/linux gate on the plannotator-tui build.
§1 requires that Herdr 0.8.2 reject the variant for its minimum while 0.9.0
accepts it, on isolated installs. Neither is visible to the manifest test, being
Herdr's behaviour rather than the file's contents, so this drives two pinned
releases and asserts each.

Both run against their own XDG_CONFIG_HOME, XDG_STATE_HOME and socket paths, so
neither can reach the machine's own server or each other's, and the plugin root
contains spaces. Verified locally on Windows 11 x86_64: the developer's running
0.9.0 server and its installed plugin were unchanged afterwards.

0.9.0 is pinned by SHA-256 the way 0.8.2 already was. RUNNER_TEMP falls back to
TEMP so the script runs unchanged outside CI, which is what recording §4's human
qualification will need.

Two observations the assertions had to accommodate.

The checklist names `plugin_requires_newer_herdr` as the rejection. What 0.8.2's
`plugin link` actually prints is unstructured:

  Error: Custom { kind: Other, error: "plugin requires Herdr 0.9.0 or newer;
  current Herdr is 0.8.2" }

Both spellings are accepted, the observed one is echoed rather than hidden
behind a pass, and the run additionally asserts nothing linked. If the code is
meant to surface on this path, that looks like a Herdr-side gap rather than a
manifest one.

An entry with no platform gate carries no `platforms` key at all -- Herdr omits
it rather than echoing the manifest default -- so under StrictMode it is a
missing property rather than an empty one, and is read through PSObject. That
absence is the assertion: every action, the doc pane and the link handler must
be effective on Windows, since an inherited Unix gate would leave the variant
installed with its review half silently unreachable.
@timo-tigges

Copy link
Copy Markdown
Author

Pushed two more commits covering the §1 version-gate boxes.

scripts/test-herdr-windows-full-plugin.ps1 drives pinned Herdr 0.8.2 and 0.9.0, each against its own XDG_CONFIG_HOME, XDG_STATE_HOME and socket paths so neither reaches the other's server or the developer's, with a plugin root containing spaces. Locally on Windows 11 x86_64:

herdr 0.8.2 rejects windows-full: plugin requires Herdr 0.9.0 or newer; current Herdr is 0.8.2
herdr 0.9.0 accepts windows-full: 7 actions, 3 panes, direct-argv doc pane

My running 0.9.0 server and its installed plugin were unchanged afterwards.

A third question, smaller than the other two. §1 says 0.8.2 rejects the variant with plugin_requires_newer_herdr. What 0.8.2's plugin link actually prints is unstructured:

Error: Custom { kind: Other, error: "plugin requires Herdr 0.9.0 or newer; current Herdr is 0.8.2" }

The rejection itself is correct and the test also asserts nothing linked, so the box's intent is satisfied. But the code does not appear on this path. The test currently accepts either spelling. Is the code expected on plugin install rather than plugin link, or is this a Herdr-side gap worth raising at herdrdev/herdr? Happy to tighten the assertion once it is settled.

One more thing worth flagging: an entry with no platform gate carries no platforms key in plugin list --json at all — Herdr omits it rather than echoing the manifest default. That absence is what the test asserts for all seven actions, the doc pane and the link handler, since an inherited Unix gate would leave the variant installed with its review half silently unreachable.

CI is showing action_required on this PR, so none of the Windows jobs have run yet — I can only report the local runs until someone approves the workflow.

The remaining doubt about this variant was never the manifest, it was whether a
native TUI can be the pane process on Windows at all. It can, and this drives it:
a pinned Herdr 0.9.0 in an isolated config, state, socket and session, a staged
checkout whose paths contain spaces, and the variant's own build commands run
from the plugin root with both override variables absent, so the binary in the
pane is the real release rather than a fixture.

The run asserts the staged location and stamp before use, opens the doc pane
with a review folder outside the checkout as cwd, and waits for a marker
generated per run -- a stale buffer cannot satisfy it -- read back out of the
pane's own terminal. Then `q` goes in through the pane, and the pane must
disappear, the originating pane must survive, no staged plannotator-tui may
still be running, and the server's own log must show the pane exiting with
status zero. Teardown runs in finally, including when an assertion fails.

Observed locally on Windows 11 x86_64 with Herdr 0.9.0:

  staged plannotator-tui 0.8.0 at windows-full/bin, native runtime one level up
  doc pane rendered HERDRFULL-0510C65C8F7B from a review folder outside the checkout
  q closed the review pane with status zero and left no process behind

The developer's own server, session and installed plugin were unchanged
afterwards, and no herdr or plannotator-tui process was left behind.

Process identity is matched on the executable path rather than the image name,
so a developer running their own plannotator-tui neither fails the run nor
satisfies it.

Still outstanding for §3: the long-path, extended-length and UNC rows of the
path matrix, placements other than the default overlay, forced-termination exit
status, and the open/open-link/last actions against controlled context
fixtures. §4 is untouched, and its native ARM64 row is not closeable on x86_64.
@timo-tigges

Copy link
Copy Markdown
Author

The review pane works on Windows. Pushed scripts/test-herdr-windows-full-pane.ps1, which drives it end to end and asserts the whole lifecycle.

Pinned Herdr 0.9.0, isolated config/state/socket/session, a staged checkout whose paths contain spaces, and the variant's own build commands run from the plugin root with PLANNOTATOR_TUI_BIN and PLANNOTATOR_TUI_RELEASE_BASE both absent — so the binary in the pane is the real 0.8.0 release, and its location and stamp are asserted before use.

staged plannotator-tui 0.8.0 at windows-full/bin, native runtime one level up
doc pane rendered HERDRFULL-0510C65C8F7B from a review folder outside the checkout
q closed the review pane with status zero and left no process behind

The marker is generated per run and read back out of the pane's own terminal via herdr pane read, so a stale buffer cannot satisfy it. Here is what actually rendered, with the review folder outside the checkout as the pane cwd:

                                        Review ▾ (m)   Copy 0 new across 0 files (E)
 FIXTURE.md        │▍ HERDRFULL-…
                   │
                   │  Unique fixture content for this run: HERDRFULL-…
 FIXTURE.md · 0 annotations · block 1/2        drag or v select · c comment · E send · tab · q quit

After q: the pane is gone from pane list, the originating pane survives, no staged plannotator-tui is running, and the server's own log records pane.exit … code: 0. Teardown runs in finally, including when an assertion fails. Process identity is matched on executable path rather than image name, so a developer running their own TUI neither fails nor satisfies the run.

Pane boundary environment

Captured with a separate native diagnostic probe (absolute program path), per §3's allowance. Everything the box asks for is present and correct:

Variable Value
HERDR_PLUGIN_ROOT the installed variant directory, normal form, no \\?\
HERDR_PLUGIN_ID matches the plugin
HERDR_PLUGIN_ENTRYPOINT_ID dump / doc
HERDR_PLUGIN_STATE_DIR the isolated state directory
HERDR_ENV 1
HERDR_PANE_ID w1:p3distinct from focused_pane_id w1:p1 in the context JSON, so the review pane and the originating pane are not confused
HERDR_PLUGIN_CONTEXT_JSON valid, with workspace id/label/cwd, tab, focused pane and cwd
HERDR_BIN_PATH the running herdr.exe
cwd the review folder

Worth noting alongside the \\?\ finding in #43: the env var is normal-form while the resolved program argument is extended-length, so code rebuilding a path from HERDR_PLUGIN_ROOT and code reading the image path see different things.

Still outstanding

§3's long-path, extended-length and UNC path-matrix rows; placements other than the default overlay; forced-termination exit status; open/open-link/last against controlled context fixtures. §4 untouched — and its native ARM64 row is not closeable on x86_64 hardware.

CI is still action_required on this PR, so none of this has run on your runner yet; everything above is local.

The matrix reuses the pane lifecycle rather than restating it: each case shells
out to test-herdr-windows-full-pane.ps1, so a row proves exactly what the
default case proves -- real fetch, per-run marker rendered, `q`, clean teardown.
The lifecycle script gains -Checkout, -Review, -LinkPath and -HerdrExecutable
for that, all defaulting to the previous standalone behaviour.

Measured on Windows 11 22621 x86_64, LongPathsEnabled=1, Herdr 0.9.0:

  special-characters   PASS (root 131, binary 155)
  extended-length-root PASS (root 123, binary 147)
  unc-loopback         PASS (root 74, binary 98)
  long-path            LIMITATION, reported below

Three fixes to the shared PowerShell fetchers, each found by a row rather than
by inspection, and each a real defect rather than a test accommodation.

Square brackets in a checkout path. `Set-Location` and every -Path parameter
read them as wildcards, so the fetchers could not find a directory that plainly
existed. Worse, once the current directory contains brackets it is stored
escaped, and even -LiteralPath with a relative path then resolves to a name with
backticks in it. Both fetchers now anchor on their own location and use absolute
literal paths throughout, depending on the current directory for nothing.

Extended-length roots. `Split-Path` cannot parse a \\?\ path -- it reports a null
drive and returns nothing -- and `Join-Path` refuses one for the same reason.
Both are replaced with the .NET equivalents, which are prefix-agnostic.

An empty PLANNOTATOR_TUI_RELEASE_BASE was treated as a release base rather than
as absent, producing a URL with no host and an "invalid URI" that surfaced far
from its cause, through the warn-and-exit-zero contract that hides it. Presence
is now tested as non-empty. This is easy to hit: passing $null to
SetEnvironmentVariable binds as an empty string and leaves the name defined.

The long-path row is not claimed as passing. Staging now succeeds past the
legacy limit, but a 267-character image cannot be started at all: process
creation takes the image path through the MAX_PATH route and accepts no
extended-length spelling, so a staged, checksum-verified, present binary still
fails to launch. The checklist forbids resolving this by changing the root
manifest, so it is measured and reported. A related constraint sits beside it:
the manifest's build command passes a relative -File argument, and Herdr resolves
only the program against the plugin root, not the arguments, so Windows
PowerShell resolves that argument against the pane cwd under MAX_PATH. Both bear
on how long an installed plugin root may be, and both are upstream's call.

Two smaller things the rows forced. Build output is captured and shown when
staging fails, because warn-and-exit-zero makes a silent failure the normal shape
of trouble. And a process launched from a share reports its image in device form,
UNC\server\share rather than \\server\share, which is folded back before
comparing -- otherwise every UNC run looks like no process at all.

UNC is a loopback share, not a remote file server, and is recorded as such.
@timo-tigges

Copy link
Copy Markdown
Author

Pushed §3's path matrix. Each row reuses the pane lifecycle rather than restating it, so a passing row means the same thing the default case means: real fetch, per-run marker rendered, q, clean teardown.

Windows 11 22621, x86_64, LongPathsEnabled=1, Herdr 0.9.0:

special-characters   PASS (root 131, binary 155)
extended-length-root PASS (root 123, binary 147)
unc-loopback         PASS (root 74,  binary 98)
long-path            LIMITATION — not claimed as passing

special-characters is the full set the checklist names in one segment: spaces, Unicode, apostrophe, &, parentheses, $, %, brackets. extended-length-root stages through the ordinary path and hands only plugin link the \\?\ spelling, since Herdr normalises the root it stores; the reported plugin_root is asserted to be the staged directory. UNC is a loopback share, not a remote file server, and is recorded as such.

Three fetcher defects the matrix found

Each was found by a row, not by inspection, and each looks like a real defect rather than something to accommodate in a test.

Square brackets in a checkout path. Set-Location and every -Path parameter read them as wildcards, so the fetchers could not find a directory that plainly existed. Worse, once the current directory contains brackets it is stored escaped, so even -LiteralPath with a relative path resolves to a name with backticks in it. Both fetchers now anchor on $PSScriptRoot and use absolute literal paths throughout, depending on the current directory for nothing.

Extended-length roots. Split-Path cannot parse a \\?\ path — it reports a null drive and returns nothing — and Join-Path refuses one for the same reason. Both replaced with the prefix-agnostic .NET equivalents.

An empty PLANNOTATOR_TUI_RELEASE_BASE was treated as a release base rather than as absent, producing a URL with no host and an invalid URI surfacing far from its cause, through the warn-and-exit-zero contract that hides it. Presence is now tested as non-empty. Easy to hit: passing $null to SetEnvironmentVariable binds as an empty string and leaves the name defined.

All existing fetcher cases still pass (-Case LocalOverride, -Case Download, and the native fetcher's LocalOverride).

The long-path row, and a second constraint beside it

Staging now succeeds past the legacy limit, but a 267-character image cannot be started at all — process creation takes the image path through the MAX_PATH route and accepts no extended-length spelling, so a staged, checksum-verified, present binary still fails to launch.

Beside it, and arguably the more interesting one: the manifest's build command passes a relative -File argument. Herdr resolves the program against the plugin root but not the arguments, so Windows PowerShell resolves that argument against the pane cwd under MAX_PATH. Measured: with a 246-character plugin root, powershell.exe -File "..\scripts\fetch-herdr-annotate.ps1" reports the argument does not exist, while the same script runs fine by absolute path. That applies to lite/'s Windows build too, not just this variant.

Both bear on how long an installed plugin root may be. The checklist forbids resolving this by changing the root manifest, so both are measured and reported rather than patched, and the row is not claimed as passing. Happy to implement whichever direction you prefer.

Two smaller things the rows forced

Build output is now captured and shown when staging fails, since warn-and-exit-zero makes a silent failure the normal shape of trouble here. And a process launched from a share reports its image in device form — UNC\server\share rather than \\server\share — which is folded back before comparing, or every UNC run looks like no process at all.

Still outstanding

Placements other than the default overlay; forced-termination exit status; open / open-link / last against controlled context fixtures; §4 entirely, whose native ARM64 row is not closeable on x86_64.

CI remains action_required on this PR, so none of the Windows jobs have run on your runner — everything above is local, on the hardware named at the top.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant