Skip to content

patch: decide its future, and fix the abort on a mid-codepoint byte offset #375

Description

@tobert

Two things landed in the same place while stating the KernelBackend::patch
batch contract in #373. The first is a concrete bug worth fixing on its own.
The second is the larger question of what patch is for, which wants
deliberate time rather than an incremental fix.

The bug: a byte offset inside a multi-byte character aborts the kernel

PatchOp::{Insert,Delete,Replace} validate only that the offset is within
content.len(). They never check that it lands on a UTF-8 character boundary.
String::insert_str, &content[a..b], drain, and replace_range all abort
on a mid-codepoint index.

Confirmed by probe, not read off the source — PatchOp::Insert { offset: 1 }
into a file beginning with é (two bytes):

thread '...' panicked at crates/kaish-kernel/src/backend/local.rs:66:25:
assertion failed: self.is_char_boundary(idx)

An embedder that computes its own byte offset takes down the kernel instead of
receiving BackendError::InvalidOperation. The file is left untouched, so the
batch contract stated in #373 still holds — but "if an operation fails, the
error returns" does not, which is why that doc was worded to promise only that
a failing operation stops the batch before the write.

The fix is a character-boundary check in the three byte-offset arms, returning
InvalidOperation with the offset and the character it split.

A smaller sibling from the same review, unverified: line: 0 is mapped to line
1 by saturating_sub(1) rather than rejected, even though the PatchOp docs
say line numbers are 1-indexed.

The larger question: how faithful should patch be?

The concern is that a partly-faithful patch is worse than none — an agent
carries expectations from patch(1), and every place kaish quietly differs is
a place those expectations produce a wrong result rather than an error.

Known divergences today:

  • No -F / --fuzz. GNU patch takes it; kaish does not. DEFAULT_FUZZ is
    hardcoded to 2 at patch.rs:423, matching GNU's default, but there is no way
    to ask for anything else — including 0.
  • max_fuzz = 0 does not mean "here or nowhere". It still searches
    file-wide for a position. There is no way to express "apply at line 42,
    refuse otherwise", which is exactly what an anchor-based edit needs. This is
    the capability gap that reopened the edit design (see the 0.15.1 slate).

Deciding this shapes the edit work: if patch becomes faithful enough to
carry anchored edits, edit needs less; if it stays a convenience, edit
carries the exact-position contract instead.

Deferred deliberately — filing so it does not get rediscovered a third time.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions