docs(installer): review the patched region, not the whole-file hash

The patch tier refused on a whole-file hash mismatch, which is the wrong
question: the three patches touch three small regions of three large files, so
an operator who edited Logging.cs somewhere else entirely was handed a manual
patch job they did not need. Hand-modified shards are the norm, so that refusal
covered most of the audience.

Replace the single hash test with a four-rung ladder (PLAN §2.2.1), cheapest and
safest first:

  0  post-patch text already present  -> no-op, keeps re-runs idempotent
  1  whole file matches the pre-image -> apply verbatim
  2  file differs, patched region is still byte-identical -> apply at the
     matched offset
  3  anything else -> do not touch the file; print the hunk to apply by hand

Rung 2 needs no new metadata: a unified diff already carries the stock text of
the region it edits (context lines plus the '-' lines). Guardrails keep it from
becoming a fuzzy apply -- exact match with only CRLF/trailing-whitespace
normalisation, exactly one occurrence or it fails, line numbers advisory only,
and all-or-nothing per patch file so a half-patched EventSink.cs cannot happen.
install.json records which rung applied each patch, and doctor and uninstall
report it.

This retires the blanket 57.4-only version gate, so PLAN gains §2.2.2 to draw
the line the ladder does not: content matching is a mechanical guarantee about
where text lands, not a support commitment. 57.4 stays the only supported
version. A non-57.4 tree may attempt the tier, but unsupported, untested and not
guaranteed -- behind a loud banner, a prompt defaulted to no, and its own
--patches-unsupported-servuo flag, because a bare --patches can be hit by
accident in a copied script. The unsupported marker persists into install.json,
every later doctor run, and the uninstall report.

INSTALL.md gets the operator-facing half: a block-quoted warning naming the
silent-script-build failure mode, the updated prerequisite row, prompts and flag
table, and a sample run showing all three outcomes.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-04 14:17:33 -05:00
parent d5838f7d4c
commit 5a6cb58a33
2 changed files with 152 additions and 18 deletions

View File

@@ -44,7 +44,7 @@ release/start scripts. The installer never writes a launcher.
| Composition | **Published bundle manifest** (§7.1). CI names an exact, protocol-checked combination of component versions; the installer fetches it at run time and `--bundle <tag>` pins one. Component releases regenerate JSON, not the installer binary |
| Token handoff | **Print token + prefilled admin URL** at the end of the run |
| Repo | **New repo**, `RunicGateway/installer`. It deploys *both* other components, so living inside `link/` would invert the dependency |
| ServUO version | **Warn and skip.** Patches are verified against stock 57.4 only; on anything else the base install proceeds and the patch tier is skipped with a warning. Forks are the norm in a public audience — refusing outright would block most operators |
| ServUO version | **57.4 is the only supported version.** The patch tier's gate is content, not a version string: a patch applies where the lines it edits are still stock and is handed to the operator where they are not (§2.2.1). Forks and hand-edited trees are the norm in a public audience, so a non-57.4 tree is still *allowed* to attempt the tier — but **unsupported, untested and not guaranteed**, behind a loud banner, a defaulted-to-no prompt and its own opt-in flag (§2.2.2) |
| Uninstall | **Never touches the ServUO tree.** Removes uo-link and its service entry, then *prints* the overlay files to delete and the patch hunks to revert. Reverting is the operator's call |
---
@@ -96,13 +96,88 @@ most real shards are hand-modified. Therefore:
audit forwarding**.
- The `EventSink.cs` patch must warn loudly that a **core solution rebuild** is required, not just a
shard restart.
- On any ServUO version other than stock **57.4**, skip the whole tier with a warning and continue
with the base install. Do not attempt to apply unverified diffs to an unknown tree.
- Record applied patches in `install.json`, **and cache the applied `.patch` files** next to it
(`/etc/runicgateway/patches/`, `%ProgramData%\RunicGateway\patches\`). Re-runs stay idempotent,
and uninstall can print the exact hunks offline long after the release tarball is gone (§5,
Phase 4).
#### 2.2.1 A whole-file hash mismatch is not a verdict — check the region
A file-level hash compare answers "is this entire file stock?", which is the wrong question. The
patches touch three small regions of three large files; an operator who added a custom command to
`Logging.cs` or a hook to `EventSink.cs` has changed the file's hash without going anywhere near the
lines the patch edits. Refusing on the file hash alone hands most real shards a manual patch job
they did not need. So the decision is made in three rungs, cheapest and safest first, and only the
last one gives up:
| Rung | Test | Outcome |
|---|---|---|
| **0 — already applied** | The hunk's *post*-patch text appears in the file | No-op, recorded as applied. Keeps re-runs idempotent |
| **1 — file is stock** | Whole-file hash matches the patch's pre-image (`index <old>..<new>` in the diff — `git hash-object` on the target reproduces it) | Apply verbatim with `git apply` |
| **2 — region is stock** | File differs, but every hunk's stock-side region is still byte-identical | Apply hunk-by-hunk at the matched offsets |
| **3 — region is modified** | Anything else | **Do not touch the file.** Print the path, the hunks and what is lost; the operator patches by hand |
Rung 2 is the semantic review, and it needs no new metadata: a unified diff already carries the
stock text of the region it edits — the context lines plus the `-` lines *are* the pre-image. For
each hunk the installer reconstructs that block and searches the target file for it, under these
rules:
- **Exact match, not fuzzy.** Only line-ending (CRLF/LF) and trailing-whitespace normalization is
allowed. No `patch --fuzz`, no context reduction: dropping context to force a match is precisely
how a patch lands in the wrong method.
- **Exactly one occurrence, or it fails.** Zero means the region moved or was edited. More than one
means the anchor is ambiguous and the installer cannot know which the author meant. Both are
rung 3.
- **Line numbers are advisory.** The hunk header's offsets are used only to prefer the nearest
candidate when reporting; the match itself is by content, since insertions above the region shift
every number below it.
- **All-or-nothing per patch file.** If one hunk of a patch reaches rung 3, none of that patch's
hunks are applied. A half-patched `EventSink.cs` compiles against a companion `.cs` that expects
the whole thing, and a partial apply is harder for an operator to unpick than an untouched file.
- **Rung 0 is checked first and is also all-or-nothing.** A file where some hunks are already
present and others are not is a hand-merge in progress, not an idempotent re-run — that is
rung 3.
#### 2.2.2 Non-57.4 is allowed, unsupported, and must say so loudly
The rung ladder replaces the blanket ServUO-version gate. The old rule skipped the entire tier on
anything other than stock 57.4 on the grounds that unverified diffs must not be applied to an
unknown tree — but forks are the norm (§1), so that rule skipped the tier for most of the audience.
Content matching gives a stronger guarantee than a version string does: on a non-57.4 tree, rung 1
is simply unavailable (its pre-image hash cannot be trusted), the tier goes straight to rung 2, and
a hunk lands only where the surrounding lines are still character-for-character the ones the patch
was written against.
**That is a mechanical safety guarantee about where text lands. It is not a support commitment, and
the installer must never let the two be confused.** Runic Gateway is designed, built and tested
against **stock ServUO 57.4**. On anything else the patch tier is **unsupported, untested, and not
guaranteed to work** — a hunk can match textually and still be wrong for a tree whose surrounding
behaviour has diverged, and neither the shard's silent script build (§2.1) nor the installer will
tell you that. So:
- **The disclaimer is unmissable, not a footnote.** On a non-57.4 tree the tier prints a banner
before it is even offered — that 57.4 is the only supported version, that the operator is on their
own here, and that a bad outcome may not surface until the shard is running.
- **It is off by default and takes an explicit, separate yes.** The interactive prompt defaults to
**no** on a non-57.4 tree, and `--patches` alone is **not** consent: an unattended run must pass
`--patches-unsupported-servuo` as well. A flag an operator had to look up cannot be hit by
accident in a script copied from somewhere else.
- **The label follows the install.** `install.json` records the detected version and the fact that
the tier ran unsupported; `doctor` shows that row on every subsequent run, not just at install
time; and the uninstall report carries it too. An operator who inherits this shard six months
later must be able to see it without being told.
- **It is the first thing quoted back in a bug report.** The tier's summary line names the detected
version, so a pasted install log answers "which ServUO?" before anyone asks.
The version is detected and reported everywhere; it just no longer *silently* decides. A refusal
becomes an informed choice, which is the point — but it stays visibly the operator's choice.
**What the installer records.** `install.json` stores, per patch, which rung applied it
(`stock-hash`, `region-match`, `already-present`) and the hunk offsets it matched. `doctor` and
`uninstall` report that: a `region-match` apply on a modified file is a different support story from
a clean apply to a stock tree, and the operator should be able to see which one they have without
re-deriving it.
### 2.3 Config paths collide with what the sidecar actually reads
The sidecar reads `$UOLINK_CONFIG`, else `sidecar.toml` in the **working directory**
@@ -227,7 +302,7 @@ The docs must state this up front rather than let users discover it as a scary d
┌────────┴────────┐ ┌────────┴────────┐
▼ ▼ ▼ ▼
overlay sync patch tier (opt-in) binary install service registration
(never deletes) (git apply + guard) + config + data (systemd / Windows SCM)
(never deletes) (per-region rungs) + config + data (systemd / Windows SCM)
```
Each component keeps its own lifecycle. ServUO's existing startup process is untouched.
@@ -400,6 +475,16 @@ Repo work that must land before an installer can exist.
Everything in §2.2. Detect applicability, dry-run, apply, record, warn about the core rebuild, and
degrade loudly rather than silently.
The rung ladder of §2.2.1 is the bulk of the work here: parse each `.patch` into hunks, reconstruct
each hunk's pre- and post-image blocks, and resolve the file through rungs 03 before writing
anything. The unsupported-version path (§2.2.2) is part of this phase, not a later polish — the
banner, the defaulted-to-no prompt, the `--patches-unsupported-servuo` flag, and the unsupported
marker carried into `install.json`, `doctor` and the uninstall report. Two pieces carry the risk and want direct tests — the hunk parser (headers, `\ No newline
at end of file`, CRLF files) and the uniqueness rule (a region that appears twice must fail, not
pick the first). Fixtures are cheap: the three stock 57.4 files, each with a hand edit far from the
patched region (must reach rung 2), an edit inside it (must reach rung 3), and an already-patched
copy (must reach rung 0).
### Phase 4 — diagnostics and updates
`runicgateway doctor` — the command that makes the whole thing supportable:
@@ -407,7 +492,7 @@ degrade loudly rather than silently.
```
✓ ServUO found /opt/ServUO (57.4)
✓ Overlay in sync 24 files, all hashes match install.json
⚠ Patch tier 1 of 3 applied — vendor.sale unavailable
⚠ Patch tier 1 of 3 applied (region-match) — vendor.sale unavailable
✓ uo-link installed 1.1.0
✓ Service running, enabled
✓ Sidecar reachable 127.0.0.1:8080 /health ok
@@ -445,7 +530,7 @@ a clever automatic revert risks silently eating their work. It removes and it re
| Removed | uo-link binary, its service entry (systemd unit / Windows service), `install.json` and the cached patch set |
| Kept | `sidecar.toml` and `uo-link.db` (config and history survive; `--purge` to drop them) |
| **Printed, not done** | Every overlay file deployed into the ServUO tree, listed by path, for the operator to delete |
| **Printed, not done** | The exact hunks each applied patch added to `EventSink.cs`, `PlayerVendorGumps.cs`, `Logging.cs`, rendered from the cached `.patch` files, for the operator to revert by hand |
| **Printed, not done** | The exact hunks each applied patch added to `EventSink.cs`, `PlayerVendorGumps.cs`, `Logging.cs`, rendered from the cached `.patch` files — with the rung that applied each one (§2.2.1), since a `region-match` apply means the surrounding file was already the operator's — for them to revert by hand |
The printed report is also written to a file, so it survives the terminal scrollback of a long
uninstall.