From 5a6cb58a331d80b816042fb214da625ba5cb7af3 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 4 Aug 2026 14:17:33 -0500 Subject: [PATCH] docs(installer): review the patched region, not the whole-file hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- installer/INSTALL.md | 73 +++++++++++++++++++++++++++------ installer/PLAN.md | 97 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 152 insertions(+), 18 deletions(-) diff --git a/installer/INSTALL.md b/installer/INSTALL.md index ef4bca6..474624a 100644 --- a/installer/INSTALL.md +++ b/installer/INSTALL.md @@ -53,7 +53,7 @@ Only the sidecar is exposed, and only to your website. | Requirement | Detail | |---|---| | A working ServUO install | It must currently boot and compile scripts cleanly. The installer deploys onto a healthy shard; it does not repair a broken one. | -| ServUO **57.4** *(patch tier only)* | The base install works on any reasonably current ServUO. The patch tier is verified against stock 57.4 only, and is skipped with a warning on anything else. | +| ServUO **57.4** *(patch tier only)* | **57.4 is the only supported version.** The base install works on any reasonably current ServUO. The patch tier is written and tested against stock 57.4; on any other version it is **unsupported and untested** — you can still choose to run it, behind an explicit opt-in, and it applies only where the exact lines it patches are unchanged. See [§4](#4-the-patch-tier-optional). | | ServUO **stopped** | `ServUO.exe` holds a lock on `Scripts.dll` and writes `Saves/` on exit. The installer refuses to deploy under a running shard. | | Administrator / root | It writes into system directories and registers a service. | | Outbound HTTPS | To `gitea.whitlocktech.com`, to fetch the bundle and the two artifacts. Nothing inbound is needed, and no Gitea account or git client is required. | @@ -140,8 +140,9 @@ one file); `doctor`, `update` and `uninstall` are run from it later. Examples be 1. **Your ServUO root** — detected if the installer is run from inside it or from an obvious sibling, otherwise prompted. A directory qualifies only if it contains `ServUO.exe`, `Scripts/` and `Config/`. -2. **Whether to apply the patch tier** — off unless you say yes, and not offered at all if your - ServUO is not 57.4. See [§4](#4-the-patch-tier-optional). +2. **Whether to apply the patch tier** — off unless you say yes. On a ServUO that is not 57.4 the + prompt defaults to **no** and carries an unsupported-version warning you have to answer past. + See [§4](#4-the-patch-tier-optional). 3. **The hostname your website should use to reach this machine** — used only to compose the two URLs it prints at the end. The sidecar's bind address is frequently `127.0.0.1` or `0.0.0.0`, neither of which is something to hand to a website. @@ -201,7 +202,8 @@ reports "unchanged" and writes nothing. | `--verify` | `install`, `update` | Dry run. Report every change that would be made; write nothing. | | `--servuo ` | `install`, `doctor`, `update` | Name the ServUO root instead of detecting or prompting. | | `--bundle ` | `install`, `update` | Pin an exact published bundle instead of the current one. | -| `--patches` / `--no-patches` | `install` | Decide the patch tier non-interactively. `--patches` still refuses on a non-57.4 tree. | +| `--patches` / `--no-patches` | `install` | Decide the patch tier non-interactively. `--patches` never loosens the region check: patches whose target lines are not stock are reported for you to apply by hand, not forced. | +| `--patches-unsupported-servuo` | `install` | Required *in addition to* `--patches` to run the patch tier on a ServUO that is not 57.4. Unsupported and untested — see [§4](#4-the-patch-tier-optional). Ignored on 57.4. | | `--host ` | `install` | The hostname to print in the website URLs. | | `--site-url ` | `install` | Your site's base URL, for the Admin → Shard link. | | `--yes` | all | Assume the default answer to every prompt. Combine with the flags above for an unattended run. | @@ -272,15 +274,62 @@ How the installer handles it: - **Opt-in.** The base install completes without it, and declining is a supported outcome, not a degraded one. -- **Dry-run first, always.** Every patch is checked (`git apply --check`) before anything is - applied, and reported per patch. Most real shards are hand-modified; a patch that does not apply - is expected, not alarming. +- **Dry-run first, always.** Every patch is checked before anything is applied, and reported per + patch. Most real shards are hand-modified; a patch that does not apply is expected, not alarming. +- **A modified file is not automatically a refusal.** These patches touch three small regions of + three large files. If you have edited `Logging.cs` somewhere else entirely, the installer says so + and still applies the patch — it checks whether *the lines the patch edits* are still stock, not + whether the whole file is. It applies only where the surrounding lines match the patch exactly and + appear exactly once; anything less and it stops and hands you the hunk to apply by hand. It never + force-fits a patch by loosening the match. - **All or nothing per feature.** The two vendor-sale patches are one unit and are applied together - or not at all. -- **Skipped entirely on a ServUO that is not 57.4**, with a warning. Unverified diffs are never - applied to an unknown tree. + or not at all — and within a patch, if one hunk cannot be placed safely, none are. +- **Your ServUO version is reported, not decisive** — but see the warning below before running this + on anything other than 57.4. - **Recorded, and the `.patch` files cached**, so re-runs stay idempotent and `uninstall` can print - the exact hunks to revert. + the exact hunks to revert — along with how each was applied, since a patch placed into a file you + had already modified is one to look at more carefully when reverting. + +### ⚠ On any ServUO that is not 57.4: unsupported, untested, no guarantees + +> **Runic Gateway is designed, built and tested against stock ServUO 57.4.** That is the only +> supported version. +> +> On any other version — a newer release, an older one, or a fork — the patch tier is +> **UNSUPPORTED, UNTESTED, and NOT GUARANTEED TO WORK.** You may run it. If you do, you are on your +> own: it is not covered by support, and a bad outcome may not show up until your shard is live, +> because ServUO's script build reports success even when it failed and quietly keeps running the +> previous `Scripts.dll`. +> +> The installer will still refuse to place a patch anywhere the exact lines it edits have changed — +> but matching text is not the same as matching behaviour. A hunk can land correctly and still be +> wrong for a tree that has diverged around it. +> +> **Back up your ServUO tree first, and verify your shard boots and compiles afterwards.** + +Because of that, on a non-57.4 tree the tier is off by default and takes a deliberate yes: + +- the interactive prompt defaults to **no** and prints the warning above; +- `--patches` on its own is **not** enough — an unattended run must also pass + `--patches-unsupported-servuo`; +- the choice is recorded, and `doctor` keeps showing an unsupported-version row for the life of the + install — so whoever looks after this shard next can see it without being told. + +A run where the tier is selected on a shard that has been worked on looks like this: + +``` +Patch tier 2 of 3 applied + ✓ playervendor-sale-eventsink Server/EventSink.cs stock file + ✓ playervendor-sale-gump Scripts/Gumps/PlayerVendorGumps.cs + file modified, patched region stock — applied at line 1180 + ✗ commandlogging-event Scripts/Commands/Logging.cs + patched region has been modified — not applied + apply this hunk by hand, then re-run install: + /etc/runicgateway/patches/commandlogging-event.patch + + ⚠ Server/EventSink.cs changed — rebuild the core: dotnet build ServUO.sln + Without commandlogging-event: no in-game moderation audit forwarding. +``` If it is skipped or fails, you lose exactly two things — **`vendor.sale` events** and **in-game moderation audit forwarding**. Everything else works. You can apply the patches later by hand (see @@ -400,7 +449,7 @@ first thing a maintainer will want. ``` ✓ 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 diff --git a/installer/PLAN.md b/installer/PLAN.md index 9be8bcb..62a46e1 100644 --- a/installer/PLAN.md +++ b/installer/PLAN.md @@ -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 ` 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 ..` 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 0–3 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.