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

wtclaude · 2026-08-04T19:18:19Z

wtclaude commented

2026-08-04 19:18:19 +00:00

What & why

Plan-stage only — the installer crate does not exist yet (Phase 1 is next), so this changes the design and the operator guide, not code.

The problem. The patch tier refused on a whole-file hash mismatch, which answers the wrong question. The three patches touch three small regions of three large files; an operator who added a custom command to Logging.cs somewhere else entirely had changed the file's hash without going anywhere near the patched lines, and was handed a manual patch job they did not need. Hand-modified shards are the norm, so that refusal covered most of the audience.

The change — a four-rung ladder (PLAN.md §2.2.1), cheapest and safest first:

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

Rung 2 is the semantic review and needs no new metadata: a unified diff already carries the stock text of the region it edits (context lines + the - lines are the pre-image). Guardrails keep it from degenerating into a fuzzy apply:

exact match, only CRLF/trailing-whitespace normalisation — no patch --fuzz, no context reduction, since dropping context to force a match is how a patch lands in the wrong method;
exactly one occurrence or it fails (zero = region moved, more than one = ambiguous anchor);
hunk-header line numbers advisory only, matching is by content;
all-or-nothing per patch file, so a half-patched EventSink.cs against a companion .cs expecting the whole thing cannot happen.

install.json records which rung applied each patch; doctor and uninstall report it, because a region-match apply into a file the operator had already modified is a different support story from a clean apply to a stock tree.

The consequence — §2.2.2, and the reason it is its own section. The ladder retires the blanket "skip the tier unless ServUO is stock 57.4" gate, so the plan now states the line the ladder does not draw: content matching is a mechanical guarantee about where text lands, not a support commitment. A hunk can match textually and still be wrong for a tree whose behaviour has diverged, and ServUO's script build reports success even when it failed (§2.1) — so nothing will tell you.

57.4 remains the only supported version. A non-57.4 tree may still attempt the tier, but unsupported, untested and not guaranteed, behind:

an unmissable banner before the tier is even offered;
a prompt defaulted to no;
its own opt-in flag — --patches alone is not consent, unattended runs need --patches-unsupported-servuo as well, because a flag you had to look up cannot be hit by accident in a script copied from somewhere else;
an unsupported marker that persists into install.json, every later doctor run, and the uninstall report, so whoever inherits the shard sees it without being told.

INSTALL.md carries the operator-facing half: a block-quoted ⚠ warning naming the silent-build failure mode and the back-up-first instruction, plus the updated prerequisite row, prompt list, flag table, and a sample run showing all three outcomes (stock file / region-match with the line it landed on / hand-off pointing at the cached .patch).

How it was tested

Documentation only — no code, no build. Reviewed for internal consistency: every place that stated the old behaviour was updated with it (§1 decision table, §2.2, Phase 3, the doctor sample and the uninstall table in PLAN.md; the prerequisites, prompts, flags, doctor sample and §4 in INSTALL.md).

Checklist

I have read CONTRIBUTING.md.
The change builds and existing tests/checks pass locally. (N/A — Markdown only.)
I have added or updated tests/docs where it makes sense. (Phase 3 now names the fixtures the rung ladder wants: an edit far from the region → rung 2, an edit inside it → rung 3, an already-patched copy → rung 0.)
My commits are reasonably scoped with clear messages.

AI-assisted contributions (required)

No AI tools were used to produce this contribution.
AI tools were used. Tool(s): Claude Code (Opus 5). I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a Co-Authored-By / Assisted-By trailer.

License

I agree that my contribution is licensed under this project's license (GNU GPL v3.0 or later), and I have the right to contribute it.

## What & why Plan-stage only — the installer crate does not exist yet (Phase 1 is next), so this changes the design and the operator guide, not code. **The problem.** The patch tier refused on a whole-file hash mismatch, which answers the wrong question. The three patches touch three small regions of three large files; an operator who added a custom command to `Logging.cs` somewhere else entirely had changed the file's hash without going anywhere near the patched lines, and was handed a manual patch job they did not need. Hand-modified shards are the norm, so that refusal covered most of the audience. **The change — a four-rung ladder (`PLAN.md` §2.2.1), cheapest and safest first:** | Rung | Test | Outcome | |---|---|---| | 0 | post-patch text already present | no-op — keeps re-runs idempotent | | 1 | whole file matches the patch's pre-image | apply verbatim | | 2 | file differs, but the patched region is still byte-identical | apply hunk-by-hunk at the matched offset | | 3 | anything else | **do not touch the file** — print the hunk to apply by hand | Rung 2 is the semantic review and needs no new metadata: a unified diff already carries the stock text of the region it edits (context lines + the `-` lines *are* the pre-image). Guardrails keep it from degenerating into a fuzzy apply: - exact match, only CRLF/trailing-whitespace normalisation — **no `patch --fuzz`, no context reduction**, since dropping context to force a match is how a patch lands in the wrong method; - **exactly one occurrence or it fails** (zero = region moved, more than one = ambiguous anchor); - hunk-header line numbers advisory only, matching is by content; - **all-or-nothing per patch file**, so a half-patched `EventSink.cs` against a companion `.cs` expecting the whole thing cannot happen. `install.json` records which rung applied each patch; `doctor` and `uninstall` report it, because a `region-match` apply into a file the operator had already modified is a different support story from a clean apply to a stock tree. **The consequence — §2.2.2, and the reason it is its own section.** The ladder retires the blanket "skip the tier unless ServUO is stock 57.4" gate, so the plan now states the line the ladder does not draw: content matching is a mechanical guarantee about *where text lands*, not a support commitment. A hunk can match textually and still be wrong for a tree whose behaviour has diverged, and ServUO's script build reports success even when it failed (§2.1) — so nothing will tell you. **57.4 remains the only supported version.** A non-57.4 tree may still attempt the tier, but **unsupported, untested and not guaranteed**, behind: - an unmissable banner before the tier is even offered; - a prompt defaulted to **no**; - its own opt-in flag — `--patches` alone is not consent, unattended runs need `--patches-unsupported-servuo` as well, because a flag you had to look up cannot be hit by accident in a script copied from somewhere else; - an unsupported marker that persists into `install.json`, every later `doctor` run, and the uninstall report, so whoever inherits the shard sees it without being told. `INSTALL.md` carries the operator-facing half: a block-quoted ⚠ warning naming the silent-build failure mode and the back-up-first instruction, plus the updated prerequisite row, prompt list, flag table, and a sample run showing all three outcomes (stock file / region-match with the line it landed on / hand-off pointing at the cached `.patch`). ## How it was tested Documentation only — no code, no build. Reviewed for internal consistency: every place that stated the old behaviour was updated with it (§1 decision table, §2.2, Phase 3, the `doctor` sample and the uninstall table in `PLAN.md`; the prerequisites, prompts, flags, `doctor` sample and §4 in `INSTALL.md`). ## Checklist - [x] I have read [CONTRIBUTING.md](CONTRIBUTING.md). - [x] The change builds and existing tests/checks pass locally. *(N/A — Markdown only.)* - [x] I have added or updated tests/docs where it makes sense. *(Phase 3 now names the fixtures the rung ladder wants: an edit far from the region → rung 2, an edit inside it → rung 3, an already-patched copy → rung 0.)* - [x] My commits are reasonably scoped with clear messages. ## AI-assisted contributions (required) - [ ] No AI tools were used to produce this contribution. - [x] AI tools were used. Tool(s): `Claude Code (Opus 5)`. I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a `Co-Authored-By` / `Assisted-By` trailer. ## License - [x] I agree that my contribution is licensed under this project's license (**GNU GPL v3.0 or later**), and I have the right to contribute it.

wtclaude added 1 commit 2026-08-04 19:18:21 +00:00

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

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>

whitlocktech approved these changes 2026-08-04 19:19:10 +00:00

whitlocktech merged commit a98fceb4bb into main

2026-08-04 19:19:16 +00:00

whitlocktech deleted branch docs/installer-patch-region-review

2026-08-04 19:19:17 +00:00

whitlocktech referenced this issue from a commit

2026-08-04 19:19:18 +00:00

Merge pull request 'docs(installer): review the patched region, not the whole-file hash' (#89) from docs/installer-patch-region-review into main

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/docs#89